llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) <details> <summary>Changes</summary> For constexpr function templates, we immediately instantiate them upon reference. However, if the function isn't defined at the time of instantiation, even though it might be defined later, the instantiation would forever fail. This patch corrects the behavior by popping up failed instantiations through PendingInstantiations, so that we are able to instantiate them again in the future (e.g. at the end of TU.) Fixes https://github.com/llvm/llvm-project/issues/125747 --- Full diff: https://github.com/llvm/llvm-project/pull/126723.diff 4 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (+16-7) - (modified) clang/lib/Interpreter/IncrementalParser.cpp (+3-2) - (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+30-17) - (modified) clang/test/CodeGenCXX/function-template-specialization.cpp (+18-1) ``````````diff diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 472a0e25adc97..2083a2c2ca40d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -13588,12 +13588,16 @@ class Sema final : public SemaBase { class LocalEagerInstantiationScope { public: - LocalEagerInstantiationScope(Sema &S) : S(S) { + LocalEagerInstantiationScope(Sema &S, bool AtEndOfTU) + : S(S), AtEndOfTU(AtEndOfTU) { SavedPendingLocalImplicitInstantiations.swap( S.PendingLocalImplicitInstantiations); } - void perform() { S.PerformPendingInstantiations(/*LocalOnly=*/true); } + void perform() { + S.PerformPendingInstantiations(/*LocalOnly=*/true, + /*AtEndOfTU=*/AtEndOfTU); + } ~LocalEagerInstantiationScope() { assert(S.PendingLocalImplicitInstantiations.empty() && @@ -13604,6 +13608,7 @@ class Sema final : public SemaBase { private: Sema &S; + bool AtEndOfTU; std::deque<PendingImplicitInstantiation> SavedPendingLocalImplicitInstantiations; }; @@ -13626,8 +13631,8 @@ class Sema final : public SemaBase { class GlobalEagerInstantiationScope { public: - GlobalEagerInstantiationScope(Sema &S, bool Enabled) - : S(S), Enabled(Enabled) { + GlobalEagerInstantiationScope(Sema &S, bool Enabled, bool AtEndOfTU) + : S(S), Enabled(Enabled), AtEndOfTU(AtEndOfTU) { if (!Enabled) return; @@ -13641,7 +13646,8 @@ class Sema final : public SemaBase { void perform() { if (Enabled) { S.DefineUsedVTables(); - S.PerformPendingInstantiations(); + S.PerformPendingInstantiations(/*LocalOnly=*/false, + /*AtEndOfTU=*/AtEndOfTU); } } @@ -13656,7 +13662,8 @@ class Sema final : public SemaBase { S.SavedVTableUses.pop_back(); // Restore the set of pending implicit instantiations. - if (S.TUKind != TU_Prefix || !S.LangOpts.PCHInstantiateTemplates) { + if ((S.TUKind != TU_Prefix || !S.LangOpts.PCHInstantiateTemplates) && + AtEndOfTU) { assert(S.PendingInstantiations.empty() && "PendingInstantiations should be empty before it is discarded."); S.PendingInstantiations.swap(S.SavedPendingInstantiations.back()); @@ -13675,6 +13682,7 @@ class Sema final : public SemaBase { private: Sema &S; bool Enabled; + bool AtEndOfTU; }; ExplicitSpecifier instantiateExplicitSpecifier( @@ -13860,7 +13868,8 @@ class Sema final : public SemaBase { /// Performs template instantiation for all implicit template /// instantiations we have seen until this point. - void PerformPendingInstantiations(bool LocalOnly = false); + void PerformPendingInstantiations(bool LocalOnly = false, + bool AtEndOfTU = true); TemplateParameterList * SubstTemplateParams(TemplateParameterList *Params, DeclContext *Owner, diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp index e43cea1baf43a..41d6304bd5f65 100644 --- a/clang/lib/Interpreter/IncrementalParser.cpp +++ b/clang/lib/Interpreter/IncrementalParser.cpp @@ -41,8 +41,9 @@ llvm::Expected<TranslationUnitDecl *> IncrementalParser::ParseOrWrapTopLevelDecl() { // Recover resources if we crash before exiting this method. llvm::CrashRecoveryContextCleanupRegistrar<Sema> CleanupSema(&S); - Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, /*Enabled=*/true); - Sema::LocalEagerInstantiationScope LocalInstantiations(S); + Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, /*Enabled=*/true, + /*AtEndOfTU=*/true); + Sema::LocalEagerInstantiationScope LocalInstantiations(S, /*AtEndOfTU=*/true); // Add a new PTU. ASTContext &C = S.getASTContext(); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index d530ed0847ae8..13d157e2a48d1 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -2139,7 +2139,8 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) { // DR1484 clarifies that the members of a local class are instantiated as part // of the instantiation of their enclosing entity. if (D->isCompleteDefinition() && D->isLocalClass()) { - Sema::LocalEagerInstantiationScope LocalInstantiations(SemaRef); + Sema::LocalEagerInstantiationScope LocalInstantiations(SemaRef, + /*AtEndOfTU=*/false); SemaRef.InstantiateClass(D->getLocation(), Record, D, TemplateArgs, TSK_ImplicitInstantiation, @@ -5121,8 +5122,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, // This has to happen before LateTemplateParser below is called, so that // it marks vtables used in late parsed templates as used. GlobalEagerInstantiationScope GlobalInstantiations(*this, - /*Enabled=*/Recursive); - LocalEagerInstantiationScope LocalInstantiations(*this); + /*Enabled=*/Recursive, + /*AtEndOfTU=*/AtEndOfTU); + LocalEagerInstantiationScope LocalInstantiations(*this, + /*AtEndOfTU=*/AtEndOfTU); // Call the LateTemplateParser callback if there is a need to late parse // a templated function definition. @@ -5691,10 +5694,12 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, // If we're performing recursive template instantiation, create our own // queue of pending implicit instantiations that we will instantiate // later, while we're still within our own instantiation context. - GlobalEagerInstantiationScope GlobalInstantiations(*this, - /*Enabled=*/Recursive); + GlobalEagerInstantiationScope GlobalInstantiations( + *this, + /*Enabled=*/Recursive, /*AtEndOfTU=*/AtEndOfTU); LocalInstantiationScope Local(*this); - LocalEagerInstantiationScope LocalInstantiations(*this); + LocalEagerInstantiationScope LocalInstantiations(*this, + /*AtEndOfTU=*/AtEndOfTU); // Enter the scope of this instantiation. We don't use // PushDeclContext because we don't have a scope. @@ -5791,14 +5796,16 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, // queue of pending implicit instantiations that we will instantiate later, // while we're still within our own instantiation context. GlobalEagerInstantiationScope GlobalInstantiations(*this, - /*Enabled=*/Recursive); + /*Enabled=*/Recursive, + /*AtEndOfTU=*/AtEndOfTU); // Enter the scope of this instantiation. We don't use // PushDeclContext because we don't have a scope. ContextRAII PreviousContext(*this, Var->getDeclContext()); LocalInstantiationScope Local(*this); - LocalEagerInstantiationScope LocalInstantiations(*this); + LocalEagerInstantiationScope LocalInstantiations(*this, + /*AtEndOfTU=*/AtEndOfTU); VarDecl *OldVar = Var; if (Def->isStaticDataMember() && !Def->isOutOfLine()) { @@ -6548,18 +6555,20 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D, return D; } -void Sema::PerformPendingInstantiations(bool LocalOnly) { - std::deque<PendingImplicitInstantiation> delayedPCHInstantiations; +void Sema::PerformPendingInstantiations(bool LocalOnly, bool AtEndOfTU) { + std::deque<PendingImplicitInstantiation> DelayedImplicitInstantiations; while (!PendingLocalImplicitInstantiations.empty() || (!LocalOnly && !PendingInstantiations.empty())) { PendingImplicitInstantiation Inst; + bool LocalInstantiation = false; if (PendingLocalImplicitInstantiations.empty()) { Inst = PendingInstantiations.front(); PendingInstantiations.pop_front(); } else { Inst = PendingLocalImplicitInstantiations.front(); PendingLocalImplicitInstantiations.pop_front(); + LocalInstantiation = true; } // Instantiate function definitions @@ -6568,22 +6577,26 @@ void Sema::PerformPendingInstantiations(bool LocalOnly) { TSK_ExplicitInstantiationDefinition; if (Function->isMultiVersion()) { getASTContext().forEachMultiversionedFunctionVersion( - Function, [this, Inst, DefinitionRequired](FunctionDecl *CurFD) { + Function, + [this, Inst, DefinitionRequired, AtEndOfTU](FunctionDecl *CurFD) { InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, CurFD, true, - DefinitionRequired, true); + DefinitionRequired, AtEndOfTU); if (CurFD->isDefined()) CurFD->setInstantiationIsPending(false); }); } else { InstantiateFunctionDefinition(/*FIXME:*/ Inst.second, Function, true, - DefinitionRequired, true); + DefinitionRequired, AtEndOfTU); if (Function->isDefined()) Function->setInstantiationIsPending(false); } // Definition of a PCH-ed template declaration may be available only in the TU. if (!LocalOnly && LangOpts.PCHInstantiateTemplates && TUKind == TU_Prefix && Function->instantiationIsPending()) - delayedPCHInstantiations.push_back(Inst); + DelayedImplicitInstantiations.push_back(Inst); + else if (!AtEndOfTU && Function->instantiationIsPending() && + !LocalInstantiation) + DelayedImplicitInstantiations.push_back(Inst); continue; } @@ -6627,11 +6640,11 @@ void Sema::PerformPendingInstantiations(bool LocalOnly) { // Instantiate static data member definitions or variable template // specializations. InstantiateVariableDefinition(/*FIXME:*/ Inst.second, Var, true, - DefinitionRequired, true); + DefinitionRequired, AtEndOfTU); } - if (!LocalOnly && LangOpts.PCHInstantiateTemplates) - PendingInstantiations.swap(delayedPCHInstantiations); + if (!DelayedImplicitInstantiations.empty()) + PendingInstantiations.swap(DelayedImplicitInstantiations); } void Sema::PerformDependentDiagnostics(const DeclContext *Pattern, diff --git a/clang/test/CodeGenCXX/function-template-specialization.cpp b/clang/test/CodeGenCXX/function-template-specialization.cpp index 7728f3dc74624..31c78358d014c 100644 --- a/clang/test/CodeGenCXX/function-template-specialization.cpp +++ b/clang/test/CodeGenCXX/function-template-specialization.cpp @@ -1,4 +1,5 @@ -// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple %s -o - | FileCheck %s +// RUN: %clang_cc1 -emit-llvm -Wundefined-func-template -verify -triple %itanium_abi_triple %s -o - | FileCheck %s +// expected-no-diagnostics // CHECK-DAG: _ZZN7PR219047GetDataIiEERKibE1i = internal global i32 4 // CHECK-DAG: _ZZN7PR219047GetDataIiEERKibE1i_0 = internal global i32 2 @@ -43,3 +44,19 @@ const int &GetData<int>(bool b) { return i; } } + +namespace GH125747 { + +template<typename F> constexpr int visit(F f) { return f(0); } + +template <class T> int G(T t); + +int main() { return visit([](auto s) -> int { return G(s); }); } + +template <class T> int G(T t) { + return 0; +} + +// CHECK: define {{.*}} @_ZN8GH1257471GIiEEiT_ + +} `````````` </details> https://github.com/llvm/llvm-project/pull/126723 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
