[0006] Resource element type validation

[bob80905]

This spec describes how the compiler will validate resource element types.

[damyanp]

Can you put the proposal number in the PR title please?

[llvm-beanz]

Because this is stuck in my head...

I don't know the full set of requirements that we would need for RWBuffer, but some examples would be:

template<typename T>
class RWBuffer {
    // T must be a complete type.
    // this builtin exists in Clang for C++ and should work for us.
    // DXC produces this error: https://godbolt.org/z/bWonbjxTz.
    static_assert(__is_complete_type(T),
                  "type parameter must be complete type."); 

    // I coined this term in the language spec to refer to handle types like
    // resources where we don't know their size
    // (see: https://github.com/microsoft/hlsl-specs/pull/198).
    // Not implemented in Clang.
    // DXC produces this error: https://godbolt.org/z/x37K1dE1e
    static_assert(!__is_intangible_type(T),
                  "type parameter must not be an intangible type.");

    // This is from Damyan's example, where we require that all fields of the
    // flattened type be the same type.
    // Not implemented in Clang.
    // DXC produces this error: https://godbolt.org/z/GGjsbjWT3
    static_assert(__is_homogenious_aggregate(T),
                  "type parameter must be an object where all fields are the same type.");
};

[llvm-beanz]

Because I'm still thinking about this (sorry!). In a C++20-world we could do something like this:

#include <type_traits>

// This is a standin for a handle...
struct Handle {
    int &X;
};

template<typename T>
concept IsComplete = __is_complete_type(T);

// This is a standin for requiring non-intangible.
template<typename T>
concept StandardLayout = std::is_standard_layout<T>::value;

template<typename T> requires IsComplete<T> && StandardLayout<T>
class RWBuffer {
    Handle H;
};

struct Incomplete;

RWBuffer<Incomplete> I;

struct NotStandard {
    int &X;
};

RWBuffer<NotStandard> NS;

struct AlsoNotStandard {
    RWBuffer<int> B;
};

RWBuffer<AlsoNotStandard> NS;

Compiler Explorer

I used std::is_standard_layout as a stand in for !__is_intangible_type. There is some conceptual overlap, but not 100%. Still no analog for homogeneous elements composition.

[bob80905]

@beanz This is extremely useful feedback! I do think I could definitely take advantage of those builtins you mentioned, it would greatly simplify the implementation of the diagnostic function that determines whether or not the resource element type is valid (we can call it DiagnoseResourceElementType for now). However, just asserting doesn't seem to capture all the information I would like. I would definitely want to point to the specific element in a struct that is non-homogenous in the diagnostic, for example. There are probably other rules that are missing like you mentioned, so I will need to read up on how DXC defines valid resource element types. But those builtins you mentioned look like a great start.

[llvm-beanz]

I do think I could definitely take advantage of those builtins you mentioned, it would greatly simplify the implementation of the diagnostic function that determines whether or not the resource element type is valid (we can call it DiagnoseResourceElementType for now).

I think we should try not to have a custom function in Clang to do this diagnostic. Someday it would be nice to move RWBuffer out of the ExternalSemaSource into a header, and we can't do that if we rely on custom baked rules to validate the type.

However, just asserting doesn't seem to capture all the information I would like. I would definitely want to point to the specific element in a struct that is non-homogenous in the diagnostic, for example.

While that may be nice, we don't do that in DXC today, and I'm not sure that is worth the extra complexity. We should aim for a simple solution that is more general purpose if we can.

There are probably other rules that are missing like you mentioned, so I will need to read up on how DXC defines valid resource element types. But those builtins you mentioned look like a great start. 👍

[bharadwajy]

I agree with @llvm-beanz that type validation done using C++ 20 traits (and concepts, if need be) is the way to go with a goal to move the functionality to a header eventually.

[bob80905]

However, just asserting doesn't seem to capture all the information I would like. I would definitely want to point to the specific element in a struct that is non-homogenous in the diagnostic, for example.

While that may be nice, we don't do that in DXC today, and I'm not sure that is worth the extra complexity. We should aim for a simple solution that is more general purpose if we can.

I should note, in one of your examples you had:

struct differentbad {
    RWBuffer<int> B;
};

RWBuffer<differentbad> invalid2;

Which emits a very informative diagnostic:

error: 'RWBuffer' is an object and cannot be used as a type parameter RWBuffer invalid2; ^ note: usage of 'RWBuffer' found in field 'B' of type 'differentbad'

I mentioned before that this doesn't seem possible with the type_trait approach, so this design would be sacrificing some diagnostic specificity.

[llvm-beanz]

Which emits a very informative diagnostic:

error: 'RWBuffer' is an object and cannot be used as a type parameter RWBuffer invalid2; ^ note: usage of 'RWBuffer' found in field 'B' of type 'differentbad'

I mentioned before that this doesn't seem possible with the type_trait approach, so this design would be sacrificing some diagnostic specificity.

I would argue that diagnostic makes no sense. “RWBuffer” is an object? so is every struct. Why is that object different from other objects?

The real reason differentbad can’t be used as a parameter for RWBuffer is because it contains a resource handle, and resource handles can’t be read from resources because they don’t have an object representation that can be stored.

There are some limitations to the quality of concept error messages at the moment, but fixing that problem seems like a better approach than building a custom error reporting system that needs to be hard-coded in the compiler.

[farzonl]

C++ 20

Most of these type traits were added as part of c++11 is_standard_layout or exist in clang proper __is_complete_type

Do we need to use concepts to achieve the goals of this proposal? My concern is that LLVM is still on c++17 and the most recent changes to concepts will land in clang 19 which is not what most folks are building with https://clang.llvm.org/cxx_status.html

[bob80905]

C++ 20

Most of these type traits were added as part of c++11 is_standard_layout or exist in clang proper __is_complete_type

Do we need to use concepts to achieve the goals of this proposal? My concern is that LLVM is still on c++17 and the most recent changes to concepts will land in clang 19 which is not what most folks are building with https://clang.llvm.org/cxx_status.html

I don't think using concepts are absolutely necessary, and using the example Chris gave, it seems this can be implemented without using concepts at all.

[bob80905]

From just reading the spec it isn't clear what the plan is for how these new type traits will be used. Chris's comments suggest using static_assert. Is that the plan? So the errors will be static_assert style errors? Or something else?

Yes, I'll make a note in the spec that static asserts will be used on these type traits to validate the types. I think the plan will indeed be to use static asserts, and static-assert-style errors will be emitted.

[damyanp]

This isn't being actively worked on at the moment, and needs to be retargeted to wg-hlsl - so closing it.