Require complete types in some missing places (microsoft#7511)

This change adds two additional calls to Sema::RequireCompleteType, one
when evaluating unary `sizeof`, which fixes a reported issue with
templates. The second is in the DXR diagnostics where complete type
should be required but not diagnosed because ordering in the compiler is
a bit wonky when diagnosing DXR entry points.

Fixes microsoft#7510

---------

Co-authored-by: Tex Riddell <[email protected]>

Author: llvm-beanz

tools/clang/lib/AST/HlslTypes.cpp

@@ -108,6 +108,7 @@ bool IsHLSLNumericOrAggregateOfNumericType(clang::QualType type) {
 // which can't be annotated. But includes UDTs of trivially copyable data and
 // the builtin trivially copyable raytracing structs.
 bool IsHLSLCopyableAnnotatableRecord(clang::QualType QT) {
+  assert(!QT->isIncompleteType() && "Type must be complete!");
   const clang::Type *Ty = QT.getCanonicalType().getTypePtr();
   if (const RecordType *RT = dyn_cast<RecordType>(Ty)) {
     const RecordDecl *RD = RT->getDecl();

tools/clang/lib/Sema/SemaDXR.cpp

@@ -1190,7 +1190,10 @@ void DiagnoseCallableEntry(Sema &S, FunctionDecl *FD,
           << /*payload|callable*/ 1 << Param;
     QualType Ty = Param->getType().getNonReferenceType();
 
-    if (!(hlsl::IsHLSLCopyableAnnotatableRecord(Ty)))
+    // Don't diagnose incomplete type here. Function parameters are
+    // checked in Sema::CheckParmsForFunctionDef.
+    if (!S.RequireCompleteType(Param->getLocation(), Ty, 0) &&
+        !(hlsl::IsHLSLCopyableAnnotatableRecord(Ty)))
       S.Diag(Param->getLocation(), diag::err_payload_attrs_must_be_udt)
           << /*payload|attributes|callable*/ 2 << /*parameter %2|type*/ 0
           << Param;
@@ -1232,6 +1235,11 @@ void DiagnoseMissOrAnyHitEntry(Sema &S, FunctionDecl *FD,
 
     QualType Ty = Param->getType().getNonReferenceType();
 
+    // Don't diagnose here, just continue if this fails. Function parameters are
+    // checked in Sema::CheckParmsForFunctionDef.
+    if (S.RequireCompleteType(Param->getLocation(), Ty, 0))
+      continue;
+
     if (!(hlsl::IsHLSLCopyableAnnotatableRecord(Ty))) {
       S.Diag(Param->getLocation(), diag::err_payload_attrs_must_be_udt)
           << /*payload|attributes|callable*/ Idx << /*parameter %2|type*/ 0
@@ -1286,6 +1294,11 @@ void DiagnoseClosestHitEntry(Sema &S, FunctionDecl *FD,
 
     QualType Ty = Param->getType().getNonReferenceType();
 
+    // Don't diagnose here, just continue if this fails. Function parameters are
+    // checked in Sema::CheckParmsForFunctionDef.
+    if (S.RequireCompleteType(Param->getLocation(), Ty, 0))
+      continue;
+
     if (!(hlsl::IsHLSLCopyableAnnotatableRecord(Ty))) {
       S.Diag(Param->getLocation(), diag::err_payload_attrs_must_be_udt)
           << /*payload|attributes|callable*/ Idx << /*parameter %2|type*/ 0

tools/clang/lib/Sema/SemaExpr.cpp

@@ -3798,13 +3798,21 @@ static void warnOnSizeofOnArrayDecay(Sema &S, SourceLocation Loc, QualType T,
 }
 
 // HLSL Change Begins
-bool Sema::CheckHLSLUnaryExprOrTypeTraitOperand(QualType ExprType, SourceLocation Loc,
+bool Sema::CheckHLSLUnaryExprOrTypeTraitOperand(QualType ExprType,
+                                                SourceLocation Loc,
                                                 UnaryExprOrTypeTrait ExprKind) {
   assert(ExprKind == UnaryExprOrTypeTrait::UETT_SizeOf);
 
-  // "sizeof 42" is ill-defined because HLSL has literal int type which can decay to an int of any size.
-  const BuiltinType* BuiltinTy = ExprType->getAs<BuiltinType>();
-  if (BuiltinTy != nullptr && (BuiltinTy->getKind() == BuiltinType::LitInt || BuiltinTy->getKind() == BuiltinType::LitFloat)) {
+  if (RequireCompleteType(Loc, ExprType,
+                          diag::err_sizeof_alignof_incomplete_type, ExprKind,
+                          ExprType))
+    return true;
+
+  // "sizeof 42" is ill-defined because HLSL has literal int type which can
+  // decay to an int of any size.
+  const BuiltinType *BuiltinTy = ExprType->getAs<BuiltinType>();
+  if (BuiltinTy != nullptr && (BuiltinTy->getKind() == BuiltinType::LitInt ||
+                               BuiltinTy->getKind() == BuiltinType::LitFloat)) {
     Diag(Loc, diag::err_hlsl_sizeof_literal) << ExprType;
     return true;
   }

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -10947,11 +10947,13 @@ HLSLExternalSource::DeduceTemplateArgumentsForHLSL(
       }
 
       if (IsBABLoad || IsBABStore) {
-        const bool IsLegalTemplate =
-            !functionTemplateTypeArg.isNull() &&
-            hlsl::IsHLSLNumericOrAggregateOfNumericType(
-                functionTemplateTypeArg);
-        if (!IsLegalTemplate) {
+        const bool IsNull = functionTemplateTypeArg.isNull();
+        // Incomplete type is diagnosed elsewhere, so just fail if incomplete.
+        if (!IsNull &&
+            getSema()->RequireCompleteType(Loc, functionTemplateTypeArg, 0))
+          return Sema::TemplateDeductionResult::TDK_Invalid;
+        if (IsNull || !hlsl::IsHLSLNumericOrAggregateOfNumericType(
+                          functionTemplateTypeArg)) {
           getSema()->Diag(Loc, diag::err_hlsl_intrinsic_template_arg_numeric)
               << intrinsicName;
           DiagnoseTypeElements(

tools/clang/test/SemaHLSL/raytracing-entry-diags.hlsl

@@ -181,3 +181,24 @@ void callable7(inout MyPayload payload, float F) {}
 
 [shader("callable")]
 float callable8(inout MyPayload payload) {} // expected-error{{return type for 'callable' shaders must be void}}
+
+// expected-note@+1 6 {{forward declaration of 'Incomplete'}}
+struct Incomplete;
+
+// expected-error@+3{{variable has incomplete type 'Incomplete'}}
+// expected-error@+2{{variable has incomplete type '__restrict Incomplete'}}
+[shader("anyhit")]
+void anyhit_incomplete( inout Incomplete A1, Incomplete A2) { }
+
+// expected-error@+3{{variable has incomplete type 'Incomplete'}}
+// expected-error@+2{{variable has incomplete type '__restrict Incomplete'}}
+[shader("closesthit")]
+void closesthit_incomplete( inout Incomplete payload, Incomplete attr ) {}
+
+// expected-error@+2{{variable has incomplete type '__restrict Incomplete'}}
+[shader("miss")]
+void miss_incomplete( inout Incomplete payload) { }
+
+// expected-error@+2{{variable has incomplete type '__restrict Incomplete'}}
+[shader("callable")]
+void callable_incomplete(inout Incomplete payload) {}

tools/clang/test/SemaHLSL/sizeof-requires-complete-type.hlsl

@@ -0,0 +1,27 @@
+// RUN: %dxc -T lib_6_3 -verify %s
+
+struct Complete {};
+
+struct Incomplete; // expected-note{{forward declaration of 'Incomplete'}}
+template<typename T> struct CompleteTemplate {};
+
+void fn() {
+  uint s;
+  // Complete types are easy. They are complete before we get to the expression.
+  s = sizeof(Complete); // This works!
+
+  // A type may be incomplete for several reasons.
+
+  // It may be incomplete because there is only a forward declaration, which
+  // should produce an error since we can't materialize a definition.
+  s = sizeof(Incomplete); // expected-error{{invalid application of 'sizeof' to an incomplete type 'Incomplete'}}
+
+  // It may be incomplete because it is an un-instantiated template, which
+  // should work because we can just instantiate it.
+  s = sizeof(CompleteTemplate<int>); // This works!
+
+  // It may be incomplete because it is a lazy-initialized type from HLSL,
+  // which can be completed, and then will report a non-numeric type error.
+  // expected-error@+1{{invalid application of 'sizeof' to non-numeric type 'Buffer'}}
+  s = sizeof(Buffer);
+}

tools/clang/test/SemaHLSL/template-udt-load.hlsl

@@ -4,12 +4,51 @@
 ByteAddressBuffer In;
 RWBuffer<float> Out;
 
+template <typename T>
+struct Foo {
+  // expected-note@+1{{'RWBuffer<float>' field declared here}}
+  T Member;
+};
+
+template <typename T>
+struct MyTemplate {
+  T GetValue(ByteAddressBuffer srv, uint offset) {
+    // expected-error@+2{{Explicit template arguments on intrinsic Load must be a single numeric type}}
+    // expected-error@+1{{object 'RWBuffer<float>' is not allowed in builtin template parameters}}
+    return srv.Load<T>(offset);
+  }
+};
+template <typename T>
+T GetValue(uint offset) {
+  MyTemplate<T> myTemplate;
+  // expected-error@+2{{scalar, vector, or matrix expected}}
+  // expected-note@+1{{in instantiation of member function 'MyTemplate<RWBuffer<float> >::GetValue' requested here}}
+  return myTemplate.GetValue(In, offset) +
+  // expected-error@+2{{Explicit template arguments on intrinsic Load must be a single numeric type}}
+  // expected-error@+1{{object 'RWBuffer<float>' is not allowed in builtin template parameters}}
+         In.Load<Foo<T> >(offset + 4).Member;
+}
+
+// expected-note@+1{{forward declaration of 'Incomplete'}}
+struct Incomplete;
+
 [shader("compute")]
 [numthreads(1,1,1)]
 void main()
 { 
   RWBuffer<float> FB = In.Load<RWBuffer<float> >(0);
   // expected-error@-1{{Explicit template arguments on intrinsic Load must be a single numeric type}}
   // expected-error@-2{{object 'RWBuffer<float>' is not allowed in builtin template parameters}}
+
   Out[0] = FB[0];
+
+  // Ok:
+  Out[4] = GetValue<float>(4);
+  
+  // expected-note@?{{'Load' declared here}}
+  // expected-error@+1{{calling 'Load' with incomplete return type 'Incomplete'}}
+  Out[8] = In.Load<Incomplete>(8);
+
+  // expected-note@+1 2 {{in instantiation of function template specialization 'GetValue<RWBuffer<float> >' requested here}}
+  RWBuffer<float> FB2 = GetValue<RWBuffer<float> >(16);
 }