[SER] Diagnose HitObject in unsupported declaration contexts

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

@@ -7558,8 +7558,6 @@ def err_hlsl_missing_type_specifier : Error< // Patterened after err_missing_typ
   "HLSL requires a type specifier for all declarations">;
 def err_hlsl_multiple_concrete_bases : Error<
   "multiple concrete base types specified">;
-def err_hlsl_objectintemplateargument : Error<
-  "%0 is an object and cannot be used as a type parameter">;
 def err_hlsl_packoffset_requires_cbuffer : Error<
   "packoffset is only allowed in a constant buffer">;
 def warn_hlsl_packoffset_mix : Warning<
@@ -7886,6 +7884,15 @@ def err_hlsl_unsupported_long_vector
     "entry function parameters|entry function return type|"
     "patch constant function parameters|patch constant function return type|"
     "payload parameters|attributes}0 are not supported">;
+// %select options must be compatible with err_hlsl_unsupported_long_vector (same index used)
+def err_hlsl_unsupported_object_context
+    : Error<"object '%0' is not allowed in "
+    "%select{ConstantBuffers or TextureBuffers|"
+    "tessellation patches|geometry streams|node records|"
+    "cbuffers or tbuffers|user-defined struct parameter|"
+    "entry function parameters|entry function return type|"
+    "patch constant function parameters|patch constant function return type|"
+    "payload parameters|attributes|type parameters|structured buffers|global variables|groupshared variables}1">;
 def err_hlsl_logical_binop_scalar : Error<
    "operands for short-circuiting logical binary operator must be scalar, for non-scalar types use '%select{and|or}0'">;
 def err_hlsl_ternary_scalar : Error<
@@ -7970,8 +7977,6 @@ def err_hlsl_too_many_node_inputs : Error<
    "Node shader '%0' may not have more than one input record">;
 def err_hlsl_node_record_type : Error<
    "%0 is not valid as a node record type - struct/class required">;
-def err_hlsl_node_record_object : Error<
-   "object %0 may not appear in a node record">;
 def err_hlsl_array_disallowed : Error<
    "%select{entry parameter|declaration}1 of type %0 may not be an array">;
 def err_hlsl_inputpatch_size: Error<

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

@@ -59,6 +59,29 @@ bool DiagnoseNodeStructArgument(clang::Sema *self,
                                 clang::QualType ArgTy, bool &Empty,
                                 const clang::FieldDecl *FD = nullptr);
 
+// Keep this in sync with err_hlsl_unsupported_object in DiagnosticSemaKinds.td
+enum class TypeDiagContext {
+  ConstantBuffersOrTextureBuffers = 0,
+  TessellationPatches = 1,
+  GeometryStreams = 2,
+  NodeRecords = 3,
+  CBuffersOrTBuffers = 4,
+  UserDefinedStructParameter = 5,
+  EntryFunctionParameters = 6,
+  EntryFunctionReturnType = 7,
+  PatchConstantFunctionParameters = 8,
+  PatchConstantFunctionReturnType = 9,
+  PayloadParameters = 10,
+  Attributes = 11,
+  TypeParameter = 12,
+  StructuredBuffers = 13,
+  GlobalVariables = 14,
+  GroupShared = 15,
+};
+bool DiagnoseTypeElements(clang::Sema &S, clang::SourceLocation Loc,
+                          clang::QualType Ty, TypeDiagContext DiagContext,
+                          const clang::FieldDecl *FD = nullptr);
+
 void DiagnoseControlFlowConditionForHLSL(clang::Sema *self,
                                          clang::Expr *condExpr,
                                          llvm::StringRef StmtName);

tools/clang/lib/AST/HlslTypes.cpp

@@ -25,6 +25,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Type.h"
 #include "clang/Sema/AttributeList.h" // conceptually ParsedAttributes
+#include "clang/Sema/SemaHLSL.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace clang;
@@ -112,6 +113,11 @@ bool IsHLSLNumericUserDefinedType(clang::QualType type) {
     for (auto member : RD->fields()) {
       if (!IsHLSLNumericOrAggregateOfNumericType(member->getType()))
         return false;
+      if (auto *Child = dyn_cast<CXXRecordDecl>(RD))
+        // Walk up the inheritance chain and check base class fields
+        for (auto &Base : Child->bases())
+          if (!IsHLSLNumericOrAggregateOfNumericType(Base.getType()))
+            return false;
     }
     return true;
   }

tools/clang/lib/Sema/SemaDXR.cpp

@@ -827,6 +827,9 @@ void DiagnoseBuiltinCallWithPayload(Sema &S, const VarDecl *Payload,
   }
 
   // Verify that the payload type is legal
+  if (DiagnoseTypeElements(S, Payload->getLocation(), Payload->getType(),
+                           TypeDiagContext::PayloadParameters))
+    return;
   if (!hlsl::IsHLSLCopyableAnnotatableRecord(Payload->getType())) {
     S.Diag(Payload->getLocation(), diag::err_payload_attrs_must_be_udt)
         << /*payload|attributes|callable*/ 0 << /*parameter %2|type*/ 0

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -46,6 +46,7 @@
 #include "clang/Sema/TemplateDeduction.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
@@ -5394,7 +5395,8 @@ class HLSLExternalSource : public ExternalSemaSource {
         objectKind = ClassifyRecordType(recordType);
         switch (objectKind) {
         case AR_TOBJ_OBJECT:
-          m_sema->Diag(argLoc, diag::err_hlsl_objectintemplateargument) << type;
+          m_sema->Diag(argLoc, diag::err_hlsl_unsupported_object_context)
+              << type << static_cast<unsigned>(TypeDiagContext::TypeParameter);
           return false;
         case AR_TOBJ_COMPOUND: {
           const RecordDecl *recordDecl = recordType->getDecl();
@@ -5539,8 +5541,24 @@ class HLSLExternalSource : public ExternalSemaSource {
               << ConstantBuffersOrTextureBuffersIdx;
           return true;
         }
+        if (DiagnoseTypeElements(
+                *m_sema, argSrcLoc, argType,
+                TypeDiagContext::ConstantBuffersOrTextureBuffers))
+          return true;
       }
       return false;
+    } else if (ResAttr && DXIL::IsStructuredBuffer(ResAttr->getResKind())) {
+      if (TemplateArgList.size() == 1) {
+        const TemplateArgumentLoc &ArgLoc = TemplateArgList[0];
+        const TemplateArgument &Arg = ArgLoc.getArgument();
+        if (Arg.getKind() == TemplateArgument::ArgKind::Type) {
+          QualType ArgType = Arg.getAsType();
+          SourceLocation ArgSrcLoc = ArgLoc.getLocation();
+          if (DiagnoseTypeElements(*m_sema, ArgSrcLoc, ArgType,
+                                   TypeDiagContext::StructuredBuffers))
+            return true;
+        }
+      }
 
     } else if (Template->getTemplatedDecl()->hasAttr<HLSLNodeObjectAttr>()) {
 
@@ -5648,6 +5666,9 @@ class HLSLExternalSource : public ExternalSemaSource {
             << TessellationPatchesIDx;
         return true;
       }
+      if (DiagnoseTypeElements(*m_sema, argLoc.getLocation(), arg.getAsType(),
+                               TypeDiagContext::TessellationPatches))
+        return true;
     } else if (Template->getTemplatedDecl()->hasAttr<HLSLStreamOutputAttr>()) {
       DXASSERT(TemplateArgList.size() > 0,
                "Geometry streams should have at least one template args");
@@ -5660,13 +5681,16 @@ class HLSLExternalSource : public ExternalSemaSource {
       CXXRecordDecl *Decl = arg.getAsType()->getAsCXXRecordDecl();
       if (Decl && !Decl->isCompleteDefinition())
         return true;
+      const unsigned GeometryStreamsIdx = 2;
       if (ContainsLongVector(arg.getAsType())) {
-        const unsigned GeometryStreamsIdx = 2;
         m_sema->Diag(argLoc.getLocation(),
                      diag::err_hlsl_unsupported_long_vector)
             << GeometryStreamsIdx;
         return true;
       }
+      if (DiagnoseTypeElements(*m_sema, argLoc.getLocation(), arg.getAsType(),
+                               TypeDiagContext::GeometryStreams))
+        return true;
     }
 
     bool isMatrix = Template->getCanonicalDecl() ==
@@ -10942,6 +10966,9 @@ HLSLExternalSource::DeduceTemplateArgumentsForHLSL(
               << intrinsicName;
           return Sema::TemplateDeductionResult::TDK_Invalid;
         }
+        if (DiagnoseTypeElements(*getSema(), Loc, functionTemplateTypeArg,
+                                 TypeDiagContext::TypeParameter))
+          return Sema::TemplateDeductionResult::TDK_Invalid;
       }
       if (IsHitObjectGetAttributes &&
           !DiagnoseIntersectionAttributes(*getSema(), Loc,
@@ -12128,34 +12155,59 @@ void Sema::DiagnoseReachableHLSLCall(CallExpr *CE, const hlsl::ShaderModel *SM,
 
 /////////////////////////////////////////////////////////////////////////////
 
-bool hlsl::DiagnoseNodeStructArgument(Sema *self, TemplateArgumentLoc ArgLoc,
-                                      QualType ArgTy, bool &Empty,
-                                      const FieldDecl *FD) {
-  DXASSERT_NOMSG(!ArgTy.isNull());
+static bool AllowObjectInContext(QualType Ty, TypeDiagContext DiagContext) {
+  // Disallow all object in template type parameters (former
+  // err_hlsl_objectintemplateargument)
+  if (DiagContext == TypeDiagContext::TypeParameter)
+    return false;
+  // Disallow all objects in node records (former
+  // err_hlsl_node_record_object)
+  if (DiagContext == TypeDiagContext::NodeRecords)
+    return false;
+  // TODO: Extend this list for other object types.
+  if (IsHLSLHitObjectType(Ty))
+    return false;
+  return true;
+}
 
-  HLSLExternalSource *source = HLSLExternalSource::FromSema(self);
-  ArTypeObjectKind shapeKind = source->GetTypeObjectKind(ArgTy);
-  switch (shapeKind) {
+static bool
+DiagnoseElementTypes(Sema &S, SourceLocation Loc, QualType Ty, bool &Empty,
+                     bool CheckLongVec, TypeDiagContext DiagContext,
+                     llvm::SmallPtrSet<const RecordDecl *, 8> &CheckedDecls,
+                     const clang::FieldDecl *FD) {
+  if (Ty.isNull() || Ty->isDependentType())
+    return false;
+
+  while (const ArrayType *Arr = Ty->getAsArrayTypeUnsafe())
+    Ty = Arr->getElementType();
+
+  const unsigned DiagContextIdx = static_cast<unsigned>(DiagContext);
+
+  HLSLExternalSource *Source = HLSLExternalSource::FromSema(&S);
+  ArTypeObjectKind ShapeKind = Source->GetTypeObjectKind(Ty);
+  switch (ShapeKind) {
   case AR_TOBJ_VECTOR:
-    if (GetHLSLVecSize(ArgTy) > DXIL::kDefaultMaxVectorLength) {
-      const unsigned NodeRecordsIdx = 3;
-      self->Diag(ArgLoc.getLocation(), diag::err_hlsl_unsupported_long_vector)
-          << NodeRecordsIdx;
+    // TODO: This is only here because DiagnoseNodeStructArgument got folded
+    // into this function. Could fold all context-dependent long vector checks
+    // into this function.
+    if (CheckLongVec && GetHLSLVecSize(Ty) > DXIL::kDefaultMaxVectorLength) {
+      S.Diag(Loc, diag::err_hlsl_unsupported_long_vector) << DiagContextIdx;
       Empty = false;
       return false;
     }
     LLVM_FALLTHROUGH;
-  case AR_TOBJ_ARRAY:
   case AR_TOBJ_BASIC:
   case AR_TOBJ_MATRIX:
     Empty = false;
     return false;
   case AR_TOBJ_OBJECT:
     Empty = false;
-    self->Diag(ArgLoc.getLocation(), diag::err_hlsl_node_record_object)
-        << ArgTy << ArgLoc.getSourceRange();
+    if (AllowObjectInContext(Ty, DiagContext))
+      return false;
+    S.Diag(Loc, diag::err_hlsl_unsupported_object_context)
+        << Ty << DiagContextIdx;
     if (FD)
-      self->Diag(FD->getLocation(), diag::note_field_declared_here)
+      S.Diag(FD->getLocation(), diag::note_field_declared_here)
           << FD->getType() << FD->getSourceRange();
     return true;
   case AR_TOBJ_DEPENDENT:
@@ -12164,25 +12216,51 @@ bool hlsl::DiagnoseNodeStructArgument(Sema *self, TemplateArgumentLoc ArgLoc,
     return true;
   case AR_TOBJ_COMPOUND: {
     bool ErrorFound = false;
-    const RecordDecl *RD = ArgTy->getAs<RecordType>()->getDecl();
+    const RecordDecl *RD = Ty->getAs<RecordType>()->getDecl();
+    // Never recurse infinitely into related subtypes
+    if (!CheckedDecls.insert(RD).second)
+      return false;
+
     // Check the fields of the RecordDecl
-    for (auto *FD : RD->fields())
+    for (auto *ElemFD : RD->fields()) {
       ErrorFound |=
-          DiagnoseNodeStructArgument(self, ArgLoc, FD->getType(), Empty, FD);
-    if (RD->isCompleteDefinition())
-      if (auto *Child = dyn_cast<CXXRecordDecl>(RD))
-        // Walk up the inheritance chain and check base class fields
-        for (auto &B : Child->bases())
-          ErrorFound |=
-              DiagnoseNodeStructArgument(self, ArgLoc, B.getType(), Empty);
+          DiagnoseElementTypes(S, Loc, ElemFD->getType(), Empty, CheckLongVec,
+                               DiagContext, CheckedDecls, ElemFD);
+    }
+    if (!RD->isCompleteDefinition())
+      return ErrorFound;
+
+    if (auto *Child = dyn_cast<CXXRecordDecl>(RD))
+      // Walk up the inheritance chain and check base class fields
+      for (auto &B : Child->bases())
+        ErrorFound |=
+            DiagnoseElementTypes(S, Loc, B.getType(), Empty, CheckLongVec,
+                                 DiagContext, CheckedDecls, nullptr);
     return ErrorFound;
   }
   default:
-    DXASSERT(false, "unreachable");
     return false;
   }
 }
 
+bool hlsl::DiagnoseTypeElements(Sema &S, SourceLocation Loc, QualType Ty,
+                                TypeDiagContext DiagContext,
+                                const clang::FieldDecl *FD) {
+  bool Empty = false;
+  llvm::SmallPtrSet<const RecordDecl *, 8> CheckedDecls;
+  return DiagnoseElementTypes(S, Loc, Ty, Empty, false /*CheckLongVec*/,
+                              DiagContext, CheckedDecls, FD);
+}
+
+bool hlsl::DiagnoseNodeStructArgument(Sema *self, TemplateArgumentLoc ArgLoc,
+                                      QualType ArgTy, bool &Empty,
+                                      const FieldDecl *FD) {
+  llvm::SmallPtrSet<const RecordDecl *, 8> CheckedDecls;
+  return DiagnoseElementTypes(*self, ArgLoc.getLocation(), ArgTy, Empty,
+                              true /*CheckLongVec*/,
+                              TypeDiagContext::NodeRecords, CheckedDecls, FD);
+}
+
 // This function diagnoses whether or not all entry-point attributes
 // should exist on this shader stage
 void DiagnoseEntryAttrAllowedOnStage(clang::Sema *self,
@@ -15295,19 +15373,42 @@ bool Sema::DiagnoseHLSLDecl(Declarator &D, DeclContext *DC, Expr *BitWidth,
     result = false;
   }
 
-  // Disallow long vecs from $Global cbuffers.
-  if (isGlobal && !isStatic && !isGroupShared && !IS_BASIC_OBJECT(basicKind)) {
+  if (isGroupShared) {
     // Suppress actual emitting of errors for incompletable types here
     // They are redundant to those produced in ActOnUninitializedDecl.
     struct SilentDiagnoser : public TypeDiagnoser {
       SilentDiagnoser() : TypeDiagnoser(true) {}
       virtual void diagnose(Sema &S, SourceLocation Loc, QualType T) {}
     } SD;
     RequireCompleteType(D.getLocStart(), qt, SD);
+    if (DiagnoseTypeElements(*this, D.getLocStart(), qt,
+                             TypeDiagContext::GroupShared))
+      result = false;
+  }
+
+  // Disallow intangible HLSL objects in the global scope.
+  if (isGlobal) {
+    // Suppress actual emitting of errors for incompletable types here
+    // They are redundant to those produced in ActOnUninitializedDecl.
+    struct SilentDiagnoser : public TypeDiagnoser {
+      SilentDiagnoser() : TypeDiagnoser(true) {}
+      virtual void diagnose(Sema &S, SourceLocation Loc, QualType T) {}
+    } SD;
+    RequireCompleteType(D.getLocStart(), qt, SD);
+    TypeDiagContext DiagContext = TypeDiagContext::CBuffersOrTBuffers;
+    if (isStatic)
+      DiagContext = TypeDiagContext::GlobalVariables;
+    if (DiagnoseTypeElements(*this, D.getLocStart(), qt, DiagContext))
+      result = false;
+  }
+
+  // Disallow long vecs from $Global cbuffers.
+  if (isGlobal && !isStatic && !isGroupShared && !IS_BASIC_OBJECT(basicKind)) {
+    const unsigned CbuffersOrTbuffersIdx = 4;
     if (ContainsLongVector(qt)) {
-      unsigned CbuffersOrTbuffersIdx = 4;
       Diag(D.getLocStart(), diag::err_hlsl_unsupported_long_vector)
           << CbuffersOrTbuffersIdx;
+
       result = false;
     }
   }
@@ -16220,7 +16321,8 @@ static bool CheckUDTIntrinsicArg(Sema *S, Expr *Arg) {
         << UserDefinedStructParameterIdx;
     return true;
   }
-  return false;
+  return DiagnoseTypeElements(*S, Arg->getExprLoc(), Arg->getType(),
+                              TypeDiagContext::UserDefinedStructParameter);
 }
 
 static bool CheckIntrinsicGetAttributeAtVertex(Sema *S, FunctionDecl *FDecl,
@@ -16962,13 +17064,17 @@ void DiagnoseEntry(Sema &S, FunctionDecl *FD) {
       S.Diag(param->getLocation(), diag::err_hlsl_unsupported_long_vector)
           << EntryFunctionParametersIdx;
     }
+    hlsl::DiagnoseTypeElements(S, param->getLocation(), param->getType(),
+                               TypeDiagContext::EntryFunctionParameters);
   }
 
   if (ContainsLongVector(FD->getReturnType())) {
     const unsigned EntryFunctionReturnIdx = 7;
     S.Diag(FD->getLocation(), diag::err_hlsl_unsupported_long_vector)
         << EntryFunctionReturnIdx;
   }
+  DiagnoseTypeElements(S, FD->getLocation(), FD->getReturnType(),
+                       TypeDiagContext::EntryFunctionReturnType);
 
   DXIL::ShaderKind Stage =
       ShaderModel::KindFromFullName(shaderAttr->getStage());