Make external sema source handle type completion (#4317)

* Make external sema source handle type completion

Prior to this change the HLSL built in types are inserted into the AST
during initialization as complete types missing their methods. The
method implementations are then inserted during parsing when handling
declarators and parameter declarations of the built in types.

This method of lazy expansion does not work with HLSL templates because
template declarators in dependent contexts are not handled by the
parser, so they bypass the code path that generates method definitions
(see #4315).

This change alters how we lazy initialize types. Instead of
lazy-initializing in parsing, this change lazy initializes when the
type is required to be complete. This is accomplished by creating the
built in type objects as incomplete decls, and adding an override for
`CompleteType` to the `HLSLExternalSource`.

This change simplifies our code because we can use the type's
`IsCompleteDefinition` flag to track types that have been completed
instead of our own bitmask system.

Fixes #4315

* Nest second isCompleteDefinition check

This will trigger in most but not all cases, so I can nest it under the
condition that triggers it to have a potentially different result.

tools/clang/lib/AST/ASTContextHLSL.cpp

@@ -407,7 +407,7 @@ CXXRecordDecl* hlsl::DeclareRecordTypeWithHandle(ASTContext& context, StringRef
   BuiltinTypeDeclBuilder typeDeclBuilder(context.getTranslationUnitDecl(), name, TagDecl::TagKind::TTK_Struct);
   typeDeclBuilder.startDefinition();
   typeDeclBuilder.addField("h", GetHLSLObjectHandleType(context));
-  return typeDeclBuilder.completeDefinition();
+  return typeDeclBuilder.getRecordDecl();
 }
 
 // creates a global static constant unsigned integer with value.
@@ -725,7 +725,7 @@ CXXRecordDecl* hlsl::DeclareTemplateTypeWithHandleInDeclContext(
 
   typeDeclBuilder.addField("h", elementType);
 
-  return typeDeclBuilder.completeDefinition();
+  return typeDeclBuilder.getRecordDecl();
 }
 
 FunctionTemplateDecl* hlsl::CreateFunctionTemplateDecl(
@@ -866,7 +866,7 @@ CXXRecordDecl *hlsl::DeclareUIntTemplatedTypeWithHandleInDeclContext(
   typeDeclBuilder.addIntegerTemplateParam(templateParamName, context.UnsignedIntTy);
   typeDeclBuilder.startDefinition();
   typeDeclBuilder.addField("h", context.UnsignedIntTy); // Add an 'h' field to hold the handle.
-  return typeDeclBuilder.completeDefinition();
+  return typeDeclBuilder.getRecordDecl();
 }
 
 clang::CXXRecordDecl *
@@ -887,7 +887,7 @@ hlsl::DeclareConstantBufferViewType(clang::ASTContext &context, bool bTBuf) {
   typeDeclBuilder.addField(
       "h", context.UnsignedIntTy); // Add an 'h' field to hold the handle.
 
-  typeDeclBuilder.completeDefinition();
+  typeDeclBuilder.getRecordDecl();
 
   return templateRecordDecl;
 }
@@ -907,7 +907,7 @@ CXXRecordDecl* hlsl::DeclareRayQueryType(ASTContext& context) {
   CreateConstructorDeclaration(context, typeDeclBuilder.getRecordDecl(), context.VoidTy, {}, context.DeclarationNames.getCXXConstructorName(canQualType), false, &pConstructorDecl, &pTypeSourceInfo);
   typeDeclBuilder.getRecordDecl()->addDecl(pConstructorDecl);
 
-  return typeDeclBuilder.completeDefinition();
+  return typeDeclBuilder.getRecordDecl();
 }
 
 CXXRecordDecl* hlsl::DeclareResourceType(ASTContext& context, bool bSampler) {
@@ -920,7 +920,7 @@ CXXRecordDecl* hlsl::DeclareResourceType(ASTContext& context, bool bSampler) {
 
   typeDeclBuilder.addField("h", GetHLSLObjectHandleType(context));
 
-  CXXRecordDecl *recordDecl = typeDeclBuilder.completeDefinition();
+  CXXRecordDecl *recordDecl = typeDeclBuilder.getRecordDecl();
 
   QualType indexType = context.UnsignedIntTy;
   QualType resultType = context.getRecordType(recordDecl);

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -3098,8 +3098,6 @@ private:
   // Map from object decl to the object index.
   using ObjectTypeDeclMapType = std::array<std::pair<CXXRecordDecl*,unsigned>, _countof(g_ArBasicKindsAsTypes)+_countof(g_DeprecatedEffectObjectNames)>;
   ObjectTypeDeclMapType m_objectTypeDeclsMap;
-  // Mask for object which not has methods created.
-  uint64_t m_objectTypeLazyInitMask;
 
   UsedIntrinsicStore m_usedIntrinsics;
 
@@ -3550,7 +3548,6 @@ private:
 
     QualType float4Type = LookupVectorType(HLSLScalarType_float, 4);
     TypeSourceInfo *float4TypeSourceInfo = m_context->getTrivialTypeSourceInfo(float4Type, NoLoc);
-    m_objectTypeLazyInitMask = 0;
     unsigned effectKindIndex = 0;
     const auto *SM =
         hlsl::ShaderModel::GetByName(m_sema->getLangOpts().HLSLProfile.c_str());
@@ -3656,7 +3653,6 @@ private:
       }
       m_objectTypeDecls[i] = recordDecl;
       m_objectTypeDeclsMap[i] = std::make_pair(recordDecl, i);
-      m_objectTypeLazyInitMask |= ((uint64_t)1)<<i;
     }
 
     // Create an alias for SamplerState. 'sampler' is very commonly used.
@@ -5117,24 +5113,26 @@ public:
   /// </remarks>
   ImplicitConversionSequence TrySubscriptIndexInitialization(_In_ clang::Expr* SrcExpr, clang::QualType DestType);
 
-  void AddHLSLObjectMethodsIfNotReady(QualType qt) {
-    static_assert((sizeof(uint64_t)*8) >= _countof(g_ArBasicKindsAsTypes), "Bitmask size is too small");
-    // Everything is ready.
-    if (m_objectTypeLazyInitMask == 0)
+  void CompleteType(TagDecl *Tag) override {
+    if (Tag->isCompleteDefinition() || !isa<CXXRecordDecl>(Tag))
       return;
-    CXXRecordDecl *recordDecl = const_cast<CXXRecordDecl *>(GetRecordDeclForBuiltInOrStruct(qt->getAsCXXRecordDecl()));
+
+    CXXRecordDecl *recordDecl = cast<CXXRecordDecl>(Tag);
+    if (auto TDecl = dyn_cast<ClassTemplateSpecializationDecl>(recordDecl)) {
+      recordDecl = TDecl->getSpecializedTemplate()->getTemplatedDecl();
+
+      if (recordDecl->isCompleteDefinition())
+        return;
+    }
+
     int idx = FindObjectBasicKindIndex(recordDecl);
     // Not object type.
     if (idx == -1)
       return;
-    uint64_t bit = ((uint64_t)1)<<idx;
-    // Already created.
-    if ((m_objectTypeLazyInitMask & bit) == 0)
-      return;
-    
+
     ArBasicKind kind = g_ArBasicKindsAsTypes[idx];
     uint8_t templateArgCount = g_ArBasicKindsTemplateCount[idx];
-    
+
     int startDepth = 0;
 
     if (templateArgCount > 0) {
@@ -5147,8 +5145,7 @@ public:
     }
 
     AddObjectMethods(kind, recordDecl, startDepth);
-    // Clear the object.
-    m_objectTypeLazyInitMask &= ~bit;
+    recordDecl->completeDefinition();
   }
 
   FunctionDecl* AddHLSLIntrinsicMethod(
@@ -12688,16 +12685,12 @@ bool Sema::DiagnoseHLSLDecl(Declarator &D, DeclContext *DC, Expr *BitWidth,
       D.setInvalidType();
       return false;
     }
-    // Add methods if not ready.
-    hlslSource->AddHLSLObjectMethodsIfNotReady(qt);
   } else if (qt->isArrayType()) {
     QualType eltQt(qt->getArrayElementTypeNoTypeQual(), 0);
     while (eltQt->isArrayType())
       eltQt = QualType(eltQt->getArrayElementTypeNoTypeQual(), 0);
 
     if (hlsl::IsObjectType(this, eltQt, &bDeprecatedEffectObject)) {
-      // Add methods if not ready.
-      hlslSource->AddHLSLObjectMethodsIfNotReady(eltQt);
       bIsObject = true;
     }
   }

tools/clang/test/HLSLFileCheck/hlsl/template/InstantiateObjectMethods.hlsl

@@ -0,0 +1,24 @@
+// RUN: %dxc -T ps_6_6 -E main -HV 2021 -ast-dump %s | FileCheck %s
+template <typename T> struct MyTex2D {
+  uint heapId;
+
+  template <typename Arg0> T Load(Arg0 arg0) { return Get().Load(arg0); }
+
+  Texture2D<T> Get() { return (Texture2D<T>)ResourceDescriptorHeap[heapId]; }
+};
+
+cbuffer constantBuffer : register(b0) {
+  MyTex2D<float4> tex;
+};
+
+float4 main() : SV_Target {
+  float4 output = tex.Load(float3(0, 0, 0));
+  return output;
+}
+
+// CHECK:      CXXMemberCallExpr 0x{{[0-9a-fA-F]+}} <col:55, col:70> 'vector<float, 4>'
+// CHECK-NEXT: MemberExpr 0x{{[0-9a-fA-F]+}} <col:55, col:61> '<bound member function type>' .Load
+// CHECK-NEXT: CXXMemberCallExpr 0x{{[0-9a-fA-F]+}} <col:55, col:59> 'Texture2D<vector<float, 4> >':'Texture2D<vector<float, 4> >'
+// CHECK-NEXT: MemberExpr 0x{{[0-9a-fA-F]+}} <col:55> '<bound member function type>' .Get
+// CHECK-NEXT: CXXThisExpr 0x{{[0-9a-fA-F]+}} <col:55> 'MyTex2D<vector<float, 4>
+// >' lvalue this