https://github.com/Icohedron updated https://github.com/llvm/llvm-project/pull/216388
>From 33222fd092d08933c6e87d02a25f3bb43f22e61d Mon Sep 17 00:00:00 2001 From: Deric Cheung <[email protected]> Date: Fri, 14 Aug 2026 11:53:18 -0700 Subject: [PATCH 1/3] Fix lazy template partial specialization instantiation --- clang/lib/AST/DeclTemplate.cpp | 8 -- clang/lib/Sema/HLSLExternalSemaSource.cpp | 27 +---- clang/lib/Sema/SemaTemplateInstantiate.cpp | 7 ++ .../Resources/Textures-declaration-order.hlsl | 48 ++++++++ clang/unittests/AST/ExternalASTSourceTest.cpp | 111 ++++++++++++++++-- 5 files changed, 160 insertions(+), 41 deletions(-) create mode 100644 clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp index 3ff6e3a3221fb..93514752e5a22 100644 --- a/clang/lib/AST/DeclTemplate.cpp +++ b/clang/lib/AST/DeclTemplate.cpp @@ -991,14 +991,6 @@ ClassTemplateSpecializationDecl *ClassTemplateSpecializationDecl::Create( Context, ClassTemplateSpecialization, TK, DC, StartLoc, IdLoc, SpecializedTemplate, Args, StrictPackMatch, PrevDecl); - // If the template decl is incomplete, copy the external lexical storage from - // the base template. This allows instantiations of incomplete types to - // complete using the external AST if the template's declaration came from an - // external AST. - if (!SpecializedTemplate->getTemplatedDecl()->isCompleteDefinition()) - Result->setHasExternalLexicalStorage( - SpecializedTemplate->getTemplatedDecl()->hasExternalLexicalStorage()); - return Result; } diff --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp b/clang/lib/Sema/HLSLExternalSemaSource.cpp index 61998e5653a00..8eb8f073ca1c9 100644 --- a/clang/lib/Sema/HLSLExternalSemaSource.cpp +++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp @@ -24,7 +24,6 @@ #include "clang/Sema/Lookup.h" #include "clang/Sema/Sema.h" #include "clang/Sema/SemaHLSL.h" -#include "clang/Sema/TemplateDeduction.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallVector.h" @@ -856,31 +855,7 @@ void HLSLExternalSemaSource::onCompletion(CXXRecordDecl *Record, void HLSLExternalSemaSource::CompleteType(TagDecl *Tag) { if (!isa<CXXRecordDecl>(Tag)) return; - auto Record = cast<CXXRecordDecl>(Tag); - - // If this is a specialization, we need to get the underlying templated - // declaration and complete that. - if (auto TDecl = dyn_cast<ClassTemplateSpecializationDecl>(Record)) { - if (!isa<ClassTemplatePartialSpecializationDecl>(TDecl)) { - ClassTemplateDecl *Template = TDecl->getSpecializedTemplate(); - llvm::SmallVector<ClassTemplatePartialSpecializationDecl *, 4> Partials; - Template->getPartialSpecializations(Partials); - ClassTemplatePartialSpecializationDecl *MatchedPartial = nullptr; - for (auto *Partial : Partials) { - sema::TemplateDeductionInfo Info(TDecl->getLocation()); - if (SemaPtr->DeduceTemplateArguments(Partial, TDecl->getTemplateArgs(), - Info) == - TemplateDeductionResult::Success) { - MatchedPartial = Partial; - break; - } - } - if (MatchedPartial) - Record = MatchedPartial; - else - Record = Template->getTemplatedDecl(); - } - } + auto *Record = cast<CXXRecordDecl>(Tag); Record = Record->getCanonicalDecl(); auto It = Completions.find(Record); if (It == Completions.end()) diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 21d68f765bdaf..11481eaeabd7c 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -4184,6 +4184,13 @@ bool Sema::InstantiateClassTemplateSpecialization( if (!Pattern.isUsable()) return Pattern.isInvalid(); + // Deduction has picked the pattern this specialization will be instantiated + // from, which may be a declaration an external AST source has yet to define. + if (!Pattern.get()->isCompleteDefinition() && + Pattern.get()->hasExternalLexicalStorage()) + if (ExternalASTSource *Source = Context.getExternalSource()) + Source->CompleteType(Pattern.get()); + bool Err = InstantiateClassImpl( PointOfInstantiation, ClassTemplateSpec, Pattern.get(), getTemplateInstantiationArgs(ClassTemplateSpec), TSK, Complain); diff --git a/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl b/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl new file mode 100644 index 0000000000000..ef4acc1aed590 --- /dev/null +++ b/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl @@ -0,0 +1,48 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -finclude-default-header -verify -DSCALAR_FIRST %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -finclude-default-header -verify %s +// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -x hlsl -fsyntax-only -finclude-default-header -verify -DSCALAR_FIRST %s +// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -x hlsl -fsyntax-only -finclude-default-header -verify %s + +// Texture resource classes are declared as a primary class template, used for +// scalar element types, plus a partial specialization used for vector element +// types. Both patterns are only defined on demand by HLSLExternalSemaSource, so +// completing one of them must not prevent the other one from being completed. +// See https://github.com/llvm/llvm-project/issues/212575. + +// expected-no-diagnostics + +#ifdef SCALAR_FIRST +Texture2D<float> Tex2D; +Texture2D<float2> Tex2DVec; +RWTexture2D<float> RWTex2D; +RWTexture2D<float2> RWTex2DVec; +Texture2DArray<float> Tex2DArray; +Texture2DArray<float2> Tex2DArrayVec; +RWTexture2DArray<float> RWTex2DArray; +RWTexture2DArray<float2> RWTex2DArrayVec; +#else +Texture2D<float2> Tex2DVec; +Texture2D<float> Tex2D; +RWTexture2D<float2> RWTex2DVec; +RWTexture2D<float> RWTex2D; +Texture2DArray<float2> Tex2DArrayVec; +Texture2DArray<float> Tex2DArray; +RWTexture2DArray<float2> RWTex2DArrayVec; +RWTexture2DArray<float> RWTex2DArray; +#endif + +SamplerState Samp; + +// Use members of both the primary template and the partial specialization to +// make sure both patterns really have been completed. +export void useTextures(float2 UV, float3 UVW) { + float S = Tex2D.Sample(Samp, UV); + float2 V = Tex2DVec.Sample(Samp, UV); + RWTex2D[uint2(0, 0)] = S; + RWTex2DVec[uint2(0, 0)] = V; + + float AS = Tex2DArray.Sample(Samp, UVW); + float2 AV = Tex2DArrayVec.Sample(Samp, UVW); + RWTex2DArray[uint3(0, 0, 0)] = AS; + RWTex2DArrayVec[uint3(0, 0, 0)] = AV; +} diff --git a/clang/unittests/AST/ExternalASTSourceTest.cpp b/clang/unittests/AST/ExternalASTSourceTest.cpp index 15483ad250976..0616ac9fd092c 100644 --- a/clang/unittests/AST/ExternalASTSourceTest.cpp +++ b/clang/unittests/AST/ExternalASTSourceTest.cpp @@ -13,6 +13,8 @@ #include "clang/AST/ExternalASTSource.h" #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclTemplate.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/CompilerInvocation.h" #include "clang/Frontend/FrontendActions.h" @@ -26,14 +28,18 @@ using namespace llvm; class TestFrontendAction : public ASTFrontendAction { public: - TestFrontendAction(IntrusiveRefCntPtr<ExternalASTSource> Source) - : Source(std::move(Source)) {} + TestFrontendAction(IntrusiveRefCntPtr<ExternalASTSource> Source, + std::function<void(ASTContext &)> Inject = nullptr) + : Source(std::move(Source)), Inject(std::move(Inject)) {} private: void ExecuteAction() override { - getCompilerInstance().getASTContext().setExternalSource(Source); - getCompilerInstance().getASTContext().getTranslationUnitDecl() - ->setHasExternalVisibleStorage(); + ASTContext &Ctx = getCompilerInstance().getASTContext(); + Ctx.setExternalSource(Source); + if (Inject) + Inject(Ctx); + else + Ctx.getTranslationUnitDecl()->setHasExternalVisibleStorage(); return ASTFrontendAction::ExecuteAction(); } @@ -43,10 +49,12 @@ class TestFrontendAction : public ASTFrontendAction { } IntrusiveRefCntPtr<ExternalASTSource> Source; + std::function<void(ASTContext &)> Inject; }; bool testExternalASTSource(llvm::IntrusiveRefCntPtr<ExternalASTSource> Source, - StringRef FileContents) { + StringRef FileContents, + std::function<void(ASTContext &)> Inject = nullptr) { auto Invocation = std::make_shared<CompilerInvocation>(); Invocation->getPreprocessorOpts().addRemappedFile( @@ -62,7 +70,7 @@ bool testExternalASTSource(llvm::IntrusiveRefCntPtr<ExternalASTSource> Source, Compiler.setVirtualFileSystem(llvm::vfs::getRealFileSystem()); Compiler.createDiagnostics(); - TestFrontendAction Action(Source); + TestFrontendAction Action(Source, std::move(Inject)); return Compiler.ExecuteAction(Action); } @@ -87,3 +95,92 @@ TEST(ExternalASTSourceTest, FailedLookupOccursOnce) { llvm::makeIntrusiveRefCnt<TestSource>(Calls), "int j, k = j;")); EXPECT_EQ(1u, Calls); } + +namespace { + +/// An external source which announces, without definitions, +/// +/// template <typename T> struct A; // primary pattern +/// template <typename T> struct A<T *>; // partial specialization pattern +/// +/// and supplies the definition of whichever pattern it is asked to complete. +struct LazyTemplatePatterns : ExternalASTSource { + CXXRecordDecl *Primary = nullptr; + ClassTemplatePartialSpecializationDecl *Partial = nullptr; + unsigned PrimaryCompletions = 0; + unsigned PartialCompletions = 0; + + void inject(ASTContext &Ctx) { + TranslationUnitDecl *TU = Ctx.getTranslationUnitDecl(); + IdentifierInfo &AName = Ctx.Idents.get("A"); + + auto MakeParams = [&] { + auto *Param = TemplateTypeParmDecl::Create( + Ctx, TU, {}, {}, /*D=*/0, /*P=*/0, &Ctx.Idents.get("T"), + /*Typename=*/true, /*ParameterPack=*/false); + return TemplateParameterList::Create(Ctx, {}, {}, {Param}, {}, nullptr); + }; + + // template <typename T> struct A; + Primary = CXXRecordDecl::Create(Ctx, TagDecl::TagKind::Struct, TU, {}, {}, + &AName); + auto *Template = ClassTemplateDecl::Create( + Ctx, TU, {}, DeclarationName(&AName), MakeParams(), Primary); + Primary->setDescribedClassTemplate(Template); + Primary->setHasExternalLexicalStorage(); + TU->addDecl(Template); + + // template <typename T> struct A<T *>; + TemplateParameterList *PartialParams = MakeParams(); + TemplateArgument Arg(Ctx.getPointerType(Ctx.getTemplateTypeParmType( + /*Depth=*/0, /*Index=*/0, /*ParameterPack=*/false, + cast<TemplateTypeParmDecl>(PartialParams->getParam(0))))); + Partial = ClassTemplatePartialSpecializationDecl::Create( + Ctx, TagDecl::TagKind::Struct, TU, {}, {}, PartialParams, Template, Arg, + Ctx.getCanonicalType(Ctx.getTemplateSpecializationType( + ElaboratedTypeKeyword::Struct, TemplateName(Template), Arg, {})), + nullptr); + Partial->setHasExternalLexicalStorage(); + + // Deduction against a partial specialization reads its arguments as + // written, so they must be supplied even though nothing was written. + TemplateArgumentListInfo ArgsInfo; + ArgsInfo.addArgument(TemplateArgumentLoc( + Arg, Ctx.getTrivialTypeSourceInfo(Arg.getAsType()))); + Partial->setTemplateArgsAsWritten( + ASTTemplateArgumentListInfo::Create(Ctx, ArgsInfo)); + + TU->addDecl(Partial); + Template->AddPartialSpecialization(Partial, nullptr); + } + + void CompleteType(TagDecl *Tag) override { + auto *Record = dyn_cast<CXXRecordDecl>(Tag); + if (!Record || Record->isCompleteDefinition()) + return; + if (Record == Primary) + ++PrimaryCompletions; + else if (Record == Partial) + ++PartialCompletions; + else + return; + Record->setHasExternalLexicalStorage(false); + Record->startDefinition(); + Record->completeDefinition(); + } +}; + +} // namespace + +// An instantiation must be able to complete the pattern it is actually built +// from, whichever of the two that is, and in either order. +TEST(ExternalASTSourceTest, CompletesPatternInEitherOrder) { + for (StringRef Code : {"A<int> a; A<int *> b;", "A<int *> b; A<int> a;"}) { + auto Source = llvm::makeIntrusiveRefCnt<LazyTemplatePatterns>(); + ASSERT_TRUE(testExternalASTSource(Source, Code, [&](ASTContext &Ctx) { + Source->inject(Ctx); + })) << Code; + EXPECT_EQ(1u, Source->PrimaryCompletions) << Code; + EXPECT_EQ(1u, Source->PartialCompletions) << Code; + } +} >From 77bd8b956e7f7ed8fe0b40c8abb5b502555f24b6 Mon Sep 17 00:00:00 2001 From: Deric Cheung <[email protected]> Date: Fri, 14 Aug 2026 17:58:39 -0700 Subject: [PATCH 2/3] Add -USCALAR_FIRST on the scalar-last run lines for clarity --- clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl b/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl index ef4acc1aed590..408c9dfe0451d 100644 --- a/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl +++ b/clang/test/SemaHLSL/Resources/Textures-declaration-order.hlsl @@ -1,7 +1,7 @@ // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -finclude-default-header -verify -DSCALAR_FIRST %s -// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -finclude-default-header -verify %s +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -fsyntax-only -finclude-default-header -verify -USCALAR_FIRST %s // RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -x hlsl -fsyntax-only -finclude-default-header -verify -DSCALAR_FIRST %s -// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -x hlsl -fsyntax-only -finclude-default-header -verify %s +// RUN: %clang_cc1 -triple spirv-unknown-vulkan-library -x hlsl -fsyntax-only -finclude-default-header -verify -USCALAR_FIRST %s // Texture resource classes are declared as a primary class template, used for // scalar element types, plus a partial specialization used for vector element >From 11bb9082e2a8f575b1b04117c34990cccbc1315f Mon Sep 17 00:00:00 2001 From: Deric Cheung <[email protected]> Date: Fri, 14 Aug 2026 18:01:07 -0700 Subject: [PATCH 3/3] Add TestExternalASTSource class for test refactoring --- clang/unittests/AST/ExternalASTSourceTest.cpp | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/clang/unittests/AST/ExternalASTSourceTest.cpp b/clang/unittests/AST/ExternalASTSourceTest.cpp index 0616ac9fd092c..19bc143ddb3d3 100644 --- a/clang/unittests/AST/ExternalASTSourceTest.cpp +++ b/clang/unittests/AST/ExternalASTSourceTest.cpp @@ -25,21 +25,22 @@ using namespace clang; using namespace llvm; +struct TestExternalASTSource : public ExternalASTSource { + virtual void setupTestAST(ASTContext &Ctx) { + Ctx.getTranslationUnitDecl()->setHasExternalVisibleStorage(); + } +}; class TestFrontendAction : public ASTFrontendAction { public: - TestFrontendAction(IntrusiveRefCntPtr<ExternalASTSource> Source, - std::function<void(ASTContext &)> Inject = nullptr) - : Source(std::move(Source)), Inject(std::move(Inject)) {} + TestFrontendAction(IntrusiveRefCntPtr<TestExternalASTSource> Source) + : Source(std::move(Source)) {} private: void ExecuteAction() override { ASTContext &Ctx = getCompilerInstance().getASTContext(); Ctx.setExternalSource(Source); - if (Inject) - Inject(Ctx); - else - Ctx.getTranslationUnitDecl()->setHasExternalVisibleStorage(); + Source->setupTestAST(Ctx); return ASTFrontendAction::ExecuteAction(); } @@ -48,13 +49,12 @@ class TestFrontendAction : public ASTFrontendAction { return std::make_unique<ASTConsumer>(); } - IntrusiveRefCntPtr<ExternalASTSource> Source; - std::function<void(ASTContext &)> Inject; + IntrusiveRefCntPtr<TestExternalASTSource> Source; }; -bool testExternalASTSource(llvm::IntrusiveRefCntPtr<ExternalASTSource> Source, - StringRef FileContents, - std::function<void(ASTContext &)> Inject = nullptr) { +bool testExternalASTSource( + llvm::IntrusiveRefCntPtr<TestExternalASTSource> Source, + StringRef FileContents) { auto Invocation = std::make_shared<CompilerInvocation>(); Invocation->getPreprocessorOpts().addRemappedFile( @@ -70,13 +70,13 @@ bool testExternalASTSource(llvm::IntrusiveRefCntPtr<ExternalASTSource> Source, Compiler.setVirtualFileSystem(llvm::vfs::getRealFileSystem()); Compiler.createDiagnostics(); - TestFrontendAction Action(Source, std::move(Inject)); + TestFrontendAction Action(Source); return Compiler.ExecuteAction(Action); } // Ensure that a failed name lookup into an external source only occurs once. TEST(ExternalASTSourceTest, FailedLookupOccursOnce) { - struct TestSource : ExternalASTSource { + struct TestSource : TestExternalASTSource { TestSource(unsigned &Calls) : Calls(Calls) {} bool @@ -104,13 +104,13 @@ namespace { /// template <typename T> struct A<T *>; // partial specialization pattern /// /// and supplies the definition of whichever pattern it is asked to complete. -struct LazyTemplatePatterns : ExternalASTSource { +struct LazyTemplatePatterns : TestExternalASTSource { CXXRecordDecl *Primary = nullptr; ClassTemplatePartialSpecializationDecl *Partial = nullptr; unsigned PrimaryCompletions = 0; unsigned PartialCompletions = 0; - void inject(ASTContext &Ctx) { + void setupTestAST(ASTContext &Ctx) override { TranslationUnitDecl *TU = Ctx.getTranslationUnitDecl(); IdentifierInfo &AName = Ctx.Idents.get("A"); @@ -177,9 +177,7 @@ struct LazyTemplatePatterns : ExternalASTSource { TEST(ExternalASTSourceTest, CompletesPatternInEitherOrder) { for (StringRef Code : {"A<int> a; A<int *> b;", "A<int *> b; A<int> a;"}) { auto Source = llvm::makeIntrusiveRefCnt<LazyTemplatePatterns>(); - ASSERT_TRUE(testExternalASTSource(Source, Code, [&](ASTContext &Ctx) { - Source->inject(Ctx); - })) << Code; + ASSERT_TRUE(testExternalASTSource(Source, Code)) << Code; EXPECT_EQ(1u, Source->PrimaryCompletions) << Code; EXPECT_EQ(1u, Source->PartialCompletions) << Code; } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
