Don't add template record decl to declcontext (#5016)

CXXRecordDecls that are part of ClassTemplateDecls should not be added
to the DeclContext that the ClassTemplateDecl gets added to.

I'm a little surprised this didn't cause problems somewhere, but it
does result in the decl being dumped twice in the ast dumping. I think
it should have also resulted in conflicting decls in name lookup, but
there may be some other change somewhere that worked around that, or it
might get ignored since the decl is invalid by itself.

Either way, we can skip adding CXXRecordDecls for template types to the
DeclContext and everything works fine.

Function and Method declarations similarly don't need to be added
if they are children of FunctionTemplateDecl. This did cause duplicate
and odd diagnosticformulations

tools/clang/include/clang/AST/HlslTypes.h

@@ -462,7 +462,8 @@ clang::CXXMethodDecl* CreateObjectFunctionDeclarationWithParams(
   llvm::ArrayRef<clang::QualType> paramTypes,
   llvm::ArrayRef<clang::StringRef> paramNames,
   clang::DeclarationName declarationName,
-  bool isConst);
+  bool isConst,
+  bool isTemplateFunction = false);
 
 DXIL::ResourceClass GetResourceClassForType(const clang::ASTContext &context,
                                             clang::QualType Ty);

tools/clang/lib/AST/ASTContextHLSL.cpp

@@ -819,7 +819,8 @@ CXXMethodDecl* hlsl::CreateObjectFunctionDeclarationWithParams(
   ArrayRef<QualType> paramTypes,
   ArrayRef<StringRef> paramNames,
   DeclarationName declarationName,
-  bool isConst)
+  bool isConst,
+  bool isTemplateFunction)
 {
   DXASSERT_NOMSG(recordDecl != nullptr);
   DXASSERT_NOMSG(!resultType.isNull());
@@ -850,7 +851,10 @@ CXXMethodDecl* hlsl::CreateObjectFunctionDeclarationWithParams(
                                            parmVarDecls.size());
   }
 
-  recordDecl->addDecl(functionDecl);
+  // If this is going to be part of a template function decl, don't add it to
+  // the record because the template function decl will be added instead.
+  if (!isTemplateFunction)
+    recordDecl->addDecl(functionDecl);
 
   return functionDecl;
 }

tools/clang/lib/AST/HlslBuiltinTypeDeclBuilder.cpp

@@ -26,7 +26,6 @@ BuiltinTypeDeclBuilder::BuiltinTypeDeclBuilder(DeclContext* declContext, StringR
   IdentifierInfo& nameId = astContext.Idents.get(name, tok::TokenKind::identifier);
   m_recordDecl = CXXRecordDecl::Create(astContext, tagKind, declContext, NoLoc, NoLoc, &nameId, nullptr, /* DelayTypeCreation */ true);
   m_recordDecl->setImplicit(true);
-  declContext->addDecl(m_recordDecl);
 }
 
 TemplateTypeParmDecl* BuiltinTypeDeclBuilder::addTypeTemplateParam(StringRef name, TypeSourceInfo* defaultValue) {
@@ -87,6 +86,8 @@ void BuiltinTypeDeclBuilder::startDefinition() {
     QualType T = m_templateDecl->getInjectedClassNameSpecialization();
     T = astContext.getInjectedClassNameType(m_recordDecl, T);
     assert(T->isDependentType() && "Class template type is not dependent?");
+  } else {
+    declContext->addDecl(m_recordDecl);
   }
 
   m_recordDecl->setLexicalDeclContext(declContext);

tools/clang/lib/Sema/SemaHLSL.cpp

@@ -3262,9 +3262,11 @@ private:
     // Create the declaration.
     IdentifierInfo* ii = &m_context->Idents.get(StringRef(intrinsic->pArgs[0].pName));
     DeclarationName declarationName = DeclarationName(ii);
-    CXXMethodDecl* functionDecl = CreateObjectFunctionDeclarationWithParams(*m_context, recordDecl,
-      functionResultQT, ArrayRef<QualType>(argsQTs, numParams), ArrayRef<StringRef>(argNames, numParams),
-      declarationName, true);
+    CXXMethodDecl *functionDecl = CreateObjectFunctionDeclarationWithParams(
+        *m_context, recordDecl, functionResultQT,
+        ArrayRef<QualType>(argsQTs, numParams),
+        ArrayRef<StringRef>(argNames, numParams), declarationName, true,
+        templateParamNamedDeclsCount > 0);
     functionDecl->setImplicit(true);
 
     // If the function is a template function, create the declaration and cross-reference.
@@ -3452,7 +3454,8 @@ private:
     CXXMethodDecl *functionDecl = CreateObjectFunctionDeclarationWithParams(
         *m_context, recordDecl, resultType, ArrayRef<QualType>(indexType),
         ArrayRef<StringRef>(StringRef("index")),
-        m_context->DeclarationNames.getCXXOperatorName(OO_Subscript), true);
+        m_context->DeclarationNames.getCXXOperatorName(OO_Subscript), true,
+        true);
     hlsl::CreateFunctionTemplateDecl(
         *m_context, recordDecl, functionDecl,
         reinterpret_cast<NamedDecl **>(&templateTypeParmDecl), 1);

tools/clang/test/HLSL/intrinsic-examples.hlsl

@@ -17,10 +17,10 @@ float4 RWByteAddressBufferMain(uint2 a : A, uint2 b : B) : SV_Target
   uint status;
   // TODO - fix the following error - the subscript exist, but the indexer type is incorrect - message is misleading
   r += uav1[b]; // expected-error {{type 'RWByteAddressBuffer' does not provide a subscript operator}} fxc-error {{X3121: array, matrix, vector, or indexable object type expected in index expression}}
-  r += uav1.Load(a); // expected-error {{no matching member function for call to 'Load'}} expected-note {{candidate function not viable: no known conversion from 'uint2' to 'TbyteOffset' for 1st argument}} expected-note {{candidate function template not viable: requires 2 arguments, but 1 was provided}} expected-note {{candidate function template not viable: requires 2 arguments, but 1 was provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 1 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
-  uav1.Load(a, status); // expected-error {{no matching member function for call to 'Load'}} expected-note {{candidate function not viable: no known conversion from 'uint2' to 'TbyteOffset' for 1st argument}} expected-note {{candidate function template not viable: requires single argument 'byteOffset', but 2 arguments were provided}} expected-note {{candidate function template not viable: requires single argument 'byteOffset', but 2 arguments were provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 2 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
+  r += uav1.Load(a); // expected-error {{no matching member function for call to 'Load'}} expected-note {{candidate function template not viable: requires 2 arguments, but 1 was provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 1 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
+  uav1.Load(a, status); // expected-error {{no matching member function for call to 'Load'}} expected-note {{candidate function template not viable: requires single argument 'byteOffset', but 2 arguments were provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 2 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
   r += status;
-  uav1.Load(a, status); // expected-error {{no matching member function for call to 'Load'}} expected-note {{candidate function not viable: no known conversion from 'uint2' to 'TbyteOffset' for 1st argument}} expected-note {{candidate function template not viable: requires single argument 'byteOffset', but 2 arguments were provided}} expected-note {{requires single argument 'byteOffset', but 2 arguments were provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 2 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
+  uav1.Load(a, status); // expected-error {{no matching member function for call to 'Load'}} expected-note {{requires single argument 'byteOffset', but 2 arguments were provided}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint)}} fxc-error {{X3013:     RWByteAddressBuffer<uint>.Load(uint, out uint status)}} fxc-error {{X3013: 'Load': no matching 2 parameter intrinsic method}} fxc-error {{X3013: Possible intrinsic methods are:}}
   r += status;
   uav1[b] = r; // expected-error {{type 'RWByteAddressBuffer' does not provide a subscript operator}} fxc-error {{X3121: array, matrix, vector, or indexable object type expected in index expression}}
   uav1.Load(a.x, status);

tools/clang/test/HLSLFileCheck/infra/ast-dumping.hlsl

@@ -22,11 +22,8 @@ void main() {
 // incomplete by looking at the `Buffer` type which is unused in this code and
 // verifying it doesn't have any methods.
 
-// IMPLICIT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class Buffer
-// IMPLICIT-NEXT: FinalAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit final
-// IMPLICIT-NEXT: FieldDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit h 'element'
-// IMPLICIT-NOT: CXXMethodDecl
-// IMPLICIT-NEXT: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit Buffer
+// IMPLICIT-NOT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class Buffer
+// IMPLICIT: ClassTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit Buffer
 // IMPLICIT-NEXT: TemplateTypeParmDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> class element
 // IMPLICIT-NEXT: TemplateArgument type 'vector<float, 4>':'vector<float, 4>'
 // IMPLICIT-NEXT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class Buffer
@@ -43,7 +40,7 @@ void main() {
 // IMPLICIT-NEXT: CXXRecordDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit class RWBuffer definition
 // IMPLICIT-NEXT: FinalAttr {{0x[0-9a-fA-F]+}} <<invalid sloc>> Implicit final
 // IMPLICIT-NEXT: FieldDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> implicit h 'element'
-// IMPLICIT-NEXT: CXXMethodDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[] 'element &(unsigned int) const'
+// IMPLICIT-NEXT: FunctionTemplateDecl {{0x[0-9a-fA-F]+}} <<invalid sloc>> <invalid sloc> operator[]
 
 // CHECK: VarDecl {{0x[0-9a-fA-F]+}} <{{.*}}ast-dumping.hlsl:3:1, col:18> col:18 used In 'RWBuffer<float3>':'RWBuffer<vector<float, 3> >'
 // CHECK-NEXT: VarDecl {{0x[0-9a-fA-F]+}} <line:4:1, col:18> col:18 used Out 'RWBuffer<float3>':'RWBuffer<vector<float, 3> >'