Fix ScoreCast for class types, fix more struct assumptions in SemaHLSL (#5110)

SemaHLSL had various locations where getAsStructureType or isStructureType were used, when classes should have been handled in the same way.

This change uses `getAs<RecordType>` instead of `getAsStructureType` and replaces `isStructureType` with `isStructureOrClassType` in locations that should handle structs and classes.

Also, it deletes unused HLSL methods in `clang::Sema` found while adding unit tests.

New tests added and existing tests updated.
- `err_hlsl_typeintemplateargument_requires_struct`: say struct/class rather than just struct
- VerifierTests with Texture2D using a specific class failed because it was `class`, not for the right reasons, other checks happen later which could happen earlier, but not this change's purpose to reimplement new diagnostics
- One test had `CHECK-NOT: and` which unfortunately collided with the branch name (`and` in `handle...`) in the llvm.ident metadata.  Added label to scope the check to the code in the function.

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7546,7 +7546,7 @@ def err_hlsl_typeintemplateargument : Error<
 def err_hlsl_typeintemplateargument_requires_scalar : Error<
   "%0 cannot be used as a type parameter where a scalar is required">;
 def err_hlsl_typeintemplateargument_requires_struct : Error<
-  "%0 cannot be used as a type parameter where a struct is required">;
+  "%0 cannot be used as a type parameter where a struct/class is required">;
 def err_hlsl_type_mismatch : Error<
   "type mismatch">;
 def err_hlsl_unsupported_array_equality_op: Error<

tools/clang/include/clang/Sema/Sema.h

@@ -4072,10 +4072,7 @@ public:
     SourceLocation LBrace);
   void ActOnFinishHLSLBuffer(Decl *Dcl, SourceLocation RBrace);
   Decl* getActiveHLSLBuffer() const;
-  void ActOnStartHLSLBufferView();
   bool IsOnHLSLBufferView();
-  Decl *ActOnHLSLBufferView(Scope *bufferScope, SourceLocation KwLoc,
-                        DeclGroupPtrTy &dcl, bool iscbuf);
   // HLSL Change Ends
 
   //===---------------------------- C++ Features --------------------------===//

tools/clang/lib/AST/HlslTypes.cpp

@@ -631,7 +631,7 @@ static bool HasTessFactorSemanticRecurse(const ValueDecl *decl, QualType Ty) {
   if (Ty->isBuiltinType() || hlsl::IsHLSLVecMatType(Ty))
     return false;
 
-  if (const RecordType *RT = Ty->getAsStructureType()) {
+  if (const RecordType *RT = Ty->getAs<RecordType>()) {
     RecordDecl *RD = RT->getDecl();
     for (FieldDecl *fieldDecl : RD->fields()) {
       if (HasTessFactorSemanticRecurse(fieldDecl, fieldDecl->getType()))

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -4933,8 +4933,8 @@ public:
         }
         return valid;
       }
-      else if (qt->isStructureType()) {
-        const RecordType* recordType = qt->getAsStructureType();
+      else if (qt->isStructureOrClassType()) {
+        const RecordType* recordType = qt->getAs<RecordType>();
         objectKind = ClassifyRecordType(recordType);
         switch (objectKind)
         {
@@ -5095,7 +5095,7 @@ public:
               llvm::ArrayRef<TemplateArgument>(TST->getArgs(),
                                                TST->getNumArgs()));
         }
-        if (const RecordType* recordType = argType->getAsStructureType()) {
+        if (const RecordType* recordType = argType->getAs<RecordType>()) {
           if (!recordType->getDecl()->isCompleteDefinition()) {
             m_sema->Diag(argSrcLoc, diag::err_typecheck_decl_incomplete_type)
                 << argType;
@@ -6963,7 +6963,7 @@ QualType HLSLExternalSource::GetNthElementType(QualType type, unsigned index) {
     return (index == 0) ? type : QualType();
   case AR_TOBJ_COMPOUND: {
     // TODO: consider caching this value for perf
-    const RecordType *recordType = type->getAsStructureType();
+    const RecordType *recordType = type->getAs<RecordType>();
     RecordDecl::field_iterator fi = recordType->getDecl()->field_begin();
     RecordDecl::field_iterator fend = recordType->getDecl()->field_end();
     while (fi != fend) {
@@ -7744,13 +7744,8 @@ bool HLSLExternalSource::IsConversionToLessOrEqualElements(
   // DerivedFrom is less.
   if (sourceTypeInfo.ShapeKind == AR_TOBJ_COMPOUND ||
       GetTypeObjectKind(sourceType) == AR_TOBJ_COMPOUND) {
-    const RecordType *targetRT = targetType->getAsStructureType();
-    if (!targetRT)
-      targetRT = dyn_cast<RecordType>(targetType);
-
-    const RecordType *sourceRT = sourceType->getAsStructureType();
-    if (!sourceRT)
-      sourceRT = dyn_cast<RecordType>(sourceType);
+    const RecordType *targetRT = targetType->getAs<RecordType>();
+    const RecordType *sourceRT = sourceType->getAs<RecordType>();
 
     if (targetRT && sourceRT) {
       RecordDecl *targetRD = targetRT->getDecl();
@@ -11584,9 +11579,8 @@ bool FlattenedTypeIterator::pushTrackerForType(QualType type, MultiExprArg::iter
     m_typeTrackers.push_back(FlattenedTypeIterator::FlattenedTypeTracker(type, 1, expression));
     return true;
   case ArTypeObjectKind::AR_TOBJ_COMPOUND: {
-    recordType = type->getAsStructureType();
-    if (recordType == nullptr)
-      recordType = dyn_cast<RecordType>(type.getTypePtr());
+    recordType = type->getAs<RecordType>();
+    DXASSERT(recordType, "compound type is expected to be a RecordType");
 
     fi = recordType->getDecl()->field_begin();
     fe = recordType->getDecl()->field_end();
@@ -11633,7 +11627,7 @@ bool FlattenedTypeIterator::pushTrackerForType(QualType type, MultiExprArg::iter
   case ArTypeObjectKind::AR_TOBJ_OBJECT: {
     if (m_source.IsSubobjectType(type)) {
       // subobjects are initialized with initialization lists
-      recordType = type->getAsStructureType();
+      recordType = type->getAs<RecordType>();
       fi = recordType->getDecl()->field_begin();
       fe = recordType->getDecl()->field_end();
 
@@ -12781,63 +12775,10 @@ Decl* Sema::getActiveHLSLBuffer() const
   return HLSLBuffers.empty() ? nullptr : HLSLBuffers.back();
 }
 
-Decl *Sema::ActOnHLSLBufferView(Scope *bufferScope, SourceLocation KwLoc,
-                            DeclGroupPtrTy &dcl, bool iscbuf) {
-  DXASSERT(nullptr == HLSLBuffers.back(), "otherwise push/pop is incorrect");
-  HLSLBuffers.pop_back();
-  DXASSERT(HLSLBuffers.empty(), "otherwise push/pop is incorrect");
-
-  Decl *decl = dcl.get().getSingleDecl();
-  NamedDecl *namedDecl = cast<NamedDecl>(decl);
-  IdentifierInfo *Ident = namedDecl->getIdentifier();
-
-  // No anonymous namespace for ConstantBuffer, take the location of the decl.
-  SourceLocation Loc = decl->getLocation();
-
-  // Prevent array type in template.  The only way to specify an array in the template type
-  // is to use a typedef, so we will strip non-typedef arrays off, since these are the legal
-  // array dimensions for the CBV/TBV, and if any array type remains, that is illegal.
-  QualType declType = cast<VarDecl>(namedDecl)->getType();
-  while (declType->isArrayType() && declType->getTypeClass() != Type::TypeClass::Typedef) {
-    const ArrayType *arrayType = declType->getAsArrayTypeUnsafe();
-    declType = arrayType->getElementType();
-  }
-  // Check to make that sure only structs are allowed as parameter types for
-  // ConstantBuffer and TextureBuffer.
-  if (!declType->isStructureType()) {
-    Diag(decl->getLocStart(),
-         diag::err_hlsl_typeintemplateargument_requires_struct)
-        << declType;
-    return nullptr;
-  }
-
-  std::vector<hlsl::UnusualAnnotation *> hlslAttrs;
-
-  DeclContext *lexicalParent = getCurLexicalContext();
-  clang::HLSLBufferDecl *result = HLSLBufferDecl::Create(
-      Context, lexicalParent, iscbuf, /*isConstantBufferView*/ true,
-      KwLoc, Ident, Loc, hlslAttrs, Loc);
-
-  // set relation
-  namedDecl->setDeclContext(result);
-  result->addDecl(namedDecl);
-  // move attribute from constant to constant buffer
-  result->setUnusualAnnotations(namedDecl->getUnusualAnnotations());
-  namedDecl->setUnusualAnnotations(hlslAttrs);
-
-  return result;
-}
-
 bool Sema::IsOnHLSLBufferView() {
   // nullptr will not pushed for cbuffer.
   return !HLSLBuffers.empty() && getActiveHLSLBuffer() == nullptr;
 }
-void Sema::ActOnStartHLSLBufferView() {
-  // Push nullptr to mark HLSLBufferView.
-  DXASSERT(HLSLBuffers.empty(), "otherwise push/pop is incorrect");
-  HLSLBuffers.emplace_back(nullptr);
-}
-
 HLSLBufferDecl::HLSLBufferDecl(
     DeclContext *DC, bool cbuffer, bool cbufferView, SourceLocation KwLoc,
     IdentifierInfo *Id, SourceLocation IdLoc,

tools/clang/test/HLSL/const-default.hlsl

@@ -24,6 +24,23 @@ struct MyStruct {
 ConstantBuffer<MyStruct> g_const_buffer;
 TextureBuffer<MyStruct> g_texture_buffer;
 
+class MyClass {
+    float3 my_float3;
+    int3x4 my_int3x4;
+};
+
+ConstantBuffer<MyClass> g_const_buffer2;
+TextureBuffer<MyClass> g_texture_buffer2;
+
+struct FWDDeclStruct;
+class FWDDeclClass;
+
+// Ensure forward declared struct/class fails as expected
+ConstantBuffer<FWDDeclStruct> g_const_buffer3;              /* expected-error {{variable has incomplete type 'FWDDeclStruct'}} */
+TextureBuffer<FWDDeclStruct> g_texture_buffer3;             /* expected-error {{variable has incomplete type 'FWDDeclStruct'}} */
+ConstantBuffer<FWDDeclClass> g_const_buffer4;               /* expected-error {{variable has incomplete type 'FWDDeclClass'}} */
+TextureBuffer<FWDDeclClass> g_texture_buffer4;              /* expected-error {{variable has incomplete type 'FWDDeclClass'}} */
+
 float4 main() : SV_TARGET
 {
     g_float1 = g_float1 + 10.0;                             /* expected-error {{cannot assign to variable 'g_float1' with const-qualified type 'const float'}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
@@ -44,5 +61,10 @@ float4 main() : SV_TARGET
     g_texture_buffer.my_float3.y += 2.0;                      /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
     g_texture_buffer.my_int3x4._14 = 3;                     /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
 
+    g_const_buffer2.my_float3.x = 1.5;                      /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
+    g_const_buffer2.my_int3x4._21 -= 2;                     /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
+    g_texture_buffer2.my_float3.y += 2.0;                     /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
+    g_texture_buffer2.my_int3x4._14 = 3;                    /* expected-error {{read-only variable is not assignable}} fxc-error {{X3025: global variables are implicitly constant, enable compatibility mode to allow modification}} */
+
     return (float4)g_float1;
 }

tools/clang/test/HLSL/cpp-errors-hv2015.hlsl

@@ -576,7 +576,9 @@ void expressions()
   internal->fn();                 // expected-error {{operator is not supported}}
   local_i = (int3) { 1, 2, 3 };   // expected-error {{compound literal is unsupported in HLSL}}
 
-  Texture2D<::c_outer_fn> local_texture; // expected-error {{'::c_outer_fn' cannot be used as a type parameter}}
+  // `class` ok, but component count should be checked earlier (1 to 4 uniform components):
+  Texture2D<::c_outer_fn> local_texture;
+
   ::new local_new; // expected-error {{new' is a reserved keyword in HLSL}}
   ::template foo local_template; // expected-error {{'template' is a reserved keyword in HLSL}} expected-error {{unknown type name 'foo'}}
 

tools/clang/test/HLSL/cpp-errors.hlsl

@@ -572,7 +572,9 @@ void expressions()
   internal->fn();                 // expected-error {{operator is not supported}}
   local_i = (int3) { 1, 2, 3 };   // expected-error {{compound literal is unsupported in HLSL}}
 
-  Texture2D<::c_outer_fn> local_texture; // expected-error {{'::c_outer_fn' cannot be used as a type parameter}}
+  // `class` ok, but component count should be checked earlier (1 to 4 uniform components):
+  Texture2D<::c_outer_fn> local_texture;
+
   ::new local_new; // expected-error {{new' is a reserved keyword in HLSL}}
   ::template foo local_template; // expected-error {{'template' is a reserved keyword in HLSL}} expected-error {{unknown type name 'foo'}}
 

tools/clang/test/HLSL/template-checks.hlsl

@@ -2,7 +2,7 @@
 
 Texture2D<float4> t_float4;
 Texture2D<SamplerState> t_obj_sampler;          /* expected-error {{'SamplerState' is an object and cannot be used as a type parameter}} fxc-error {{X3124: object element type cannot be an object type}} */
-Texture2D<Texture2D<float4> > t_obj_tex;        /* expected-error {{'Texture2D<float4>' cannot be used as a type parameter}} fxc-error {{X3124: object element type cannot be an object type}} */
+Texture2D<Texture2D<float4> > t_obj_tex;        /* expected-error {{'Texture2D<float4>' is an object and cannot be used as a type parameter}} fxc-error {{X3124: object element type cannot be an object type}} */
 
 matrix<SamplerState, 1, 2> m_obj_sampler;       /* expected-error {{'SamplerState' cannot be used as a type parameter where a scalar is required}} fxc-error {{X3123: matrix element type must be a scalar type}} */
 matrix<bool, 1, 2> m_bool;

tools/clang/test/HLSLFileCheck/hlsl/classes/class_method_overload.hlsl

@@ -0,0 +1,27 @@
+// RUN: %dxc -T vs_6_0 -E main %s -ast-dump | FileCheck %s
+
+// CHECK: FunctionDecl {{.*}} used CallMethod 'uint (__restrict MyClass)'
+// CHECK: CXXMemberCallExpr
+// CHECK-NEXT: MemberExpr {{.*}} .Method
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'MyClass' lvalue <NoOp>
+// CHECK-NEXT: DeclRefExpr
+// CHECK-SAME: '__restrict MyClass' lvalue ParmVar 0x{{[0-9a-zA-Z]+}} 'c' '__restrict MyClass'
+
+class MyClass {
+  uint Method(uint2 u2) { return u2.y * 2; }
+  uint Method(uint3 u3) { return u3.y * 3; }
+  uint u;
+};
+
+uint CallMethod(inout MyClass c) {
+  // `inout MyClass c` translates to: `__restrict MyClass&`
+  // This doesn't exactly match MyClass type for `Method` call, so if method
+  // is overloaded, ScoreCast is used, leading to a comparison of MyClass type,
+  // which was broken for `class` types as opposed to `struct` types.
+  return c.Method(uint2(1,2));
+}
+
+uint main() : OUT {
+  MyClass c = (MyClass)1;
+  return CallMethod(c);
+}

tools/clang/test/HLSLFileCheck/hlsl/objects/Cbuffer/class-in-ConstantBuffer.hlsl

@@ -0,0 +1,17 @@
+// RUN: %dxc -E main -T vs_6_0 -ast-dump %s | FileCheck %s
+
+// CHECK: VarDecl 0x{{[0-9a-zA-Z]+}} {{.*}} used cbv 'ConstantBuffer<CBType>':'ConstantBuffer<CBType>'
+
+class Inner {
+  int a;
+};
+
+class CBType {
+  Inner m;
+};
+
+ConstantBuffer<CBType> cbv;
+
+int main() : OUT {
+  return cbv.m.a;
+}

tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbuffer-struct.hlsl

@@ -8,11 +8,11 @@ struct S {
     float4 f;
 };
 
-// CHECK: error: 'int' cannot be used as a type parameter where a struct is required
+// CHECK: error: 'int' cannot be used as a type parameter where a struct/class is required
 ConstantBuffer<int>      B1;
-// CHECK: error: 'float2' cannot be used as a type parameter where a struct is required
+// CHECK: error: 'float2' cannot be used as a type parameter where a struct/class is required
 TextureBuffer<float2>    B2;
-// CHECK: error: 'float3x4' cannot be used as a type parameter where a struct is required
+// CHECK: error: 'float3x4' cannot be used as a type parameter where a struct/class is required
 ConstantBuffer<float3x4> B3;
 
 TextureBuffer<C>         B4;

tools/clang/test/HLSLFileCheck/hlsl/objects/CbufferLegacy/cbuffer-structarray.hlsl

@@ -6,12 +6,12 @@ struct Foo {
 
 typedef Foo FooA[2];
 
-// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct is required
+// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct/class is required
 ConstantBuffer<FooA> CB1;
 
-// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct is required
+// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct/class is required
 ConstantBuffer<FooA> CB[4][3];
-// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct is required
+// CHECK: error: 'FooA' (aka 'Foo [2]') cannot be used as a type parameter where a struct/class is required
 TextureBuffer<FooA> TB[4][3];
 
 float4 main(int a : A) : SV_Target

tools/clang/test/HLSLFileCheck/hlsl/objects/StructuredBuffer/struct_buf_class_elt.hlsl

@@ -0,0 +1,17 @@
+// RUN: %dxc -T vs_6_0 -E main -ast-dump %s | FileCheck %s
+
+// CHECK: VarDecl 0x{{[0-9a-zA-Z]+}} {{.*}} used sb 'StructuredBuffer<Element>':'StructuredBuffer<Element>'
+// CHECK-NEXT: VarDecl 0x{{[0-9a-zA-Z]+}} {{.*}} used rwsb 'RWStructuredBuffer<Element>':'RWStructuredBuffer<Element>'
+
+class Element
+{
+    int e;
+};
+
+StructuredBuffer<Element> sb;
+RWStructuredBuffer<Element> rwsb;
+
+int main() : OUT
+{
+    return sb[0].e + rwsb[0].e;
+}

tools/clang/test/HLSLFileCheck/hlsl/types/vector/vec_uint_shr.hlsl

@@ -5,6 +5,7 @@
 // CHECK-NOT: ashr
 // Make sure no and for src1 of lshr.
 // CHECK-NOT: and
+// CHECK-LABEL: ret
 
 
 float main(uint2 a:A, uint b:B) : SV_Target {

tools/clang/test/HLSLFileCheck/shader_targets/hull/pcfunc-uses-class.hlsl

@@ -0,0 +1,47 @@
+// RUN: %dxc -E main -T hs_6_0  %s | FileCheck %s
+
+// Make sure input control point is not 0.
+// CHECK: !{void ()* @"\01?HSPerPatchFunc@@YA?AVHSPerPatchData@@XZ", i32 1
+
+// Ensure class works when detecting Patch Constant function
+class HSPerPatchData
+{
+    // We at least have to specify tess factors per patch
+    // As we're tesselating triangles, there will be 4 tess factors
+    // In real life case this might contain face normal, for example
+	float	edges[3] : SV_TessFactor;
+	float	inside   : SV_InsideTessFactor;
+};
+
+
+
+// This overload is a patch constant function candidate because it has an
+// output with the SV_TessFactor semantic. However, the compiler should
+// *not* select it because there is another overload defined later in this
+// translation unit (which is the old compiler's behavior). If it did, then
+// the semantic checker will report an error due to this overload's input
+// having 32 elements (versus the expected 3).
+HSPerPatchData HSPerPatchFunc()
+{
+  HSPerPatchData d;
+
+  d.edges[0] = -5;
+  d.edges[1] = -6;
+  d.edges[2] = -7;
+  d.inside = -8;
+
+  return d;
+}
+
+
+
+// hull per-control point shader
+[domain("tri")]
+[partitioning("fractional_odd")]
+[outputtopology("triangle_cw")]
+[patchconstantfunc("HSPerPatchFunc")]
+[outputcontrolpoints(3)]
+void main( const uint id : SV_OutputControlPointID )
+{
+}
+