From dcf5b6fd619229d0240a67815527f792c72b25ba Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sun, 5 Oct 2025 22:25:52 -0300 Subject: [PATCH] [clang] separate recursive instantiation check from CodeSynthesisContext This makes pushing / popping CodeSynthesisContexts much cheaper, as it delegates to another class this functionality which is not actually needed in most cases. It also converts a bunch of these uses into just asserts. This improves compiler performance a little bit: Some diagnostics change a little bit, because we avoid printing a redundant context notes. --- clang/include/clang/Sema/Sema.h | 37 ++++++++-- clang/lib/Sema/SemaTemplateInstantiate.cpp | 68 ++++++++++--------- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 36 ++++++---- .../test/SemaCXX/libstdcxx_pair_swap_hack.cpp | 7 +- clang/test/SemaTemplate/instantiate-self.cpp | 6 +- 5 files changed, 99 insertions(+), 55 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 265462a86e405..7456285472dd1 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12995,6 +12995,36 @@ class Sema final : public SemaBase { /// default arguments of its methods have been parsed. UnparsedDefaultArgInstantiationsMap UnparsedDefaultArgInstantiations; + using InstantiatingSpecializationsKey = llvm::PointerIntPair; + + struct RecursiveInstGuard { + enum class Kind { + Template, + DefaultArgument, + ExceptionSpec, + }; + + RecursiveInstGuard(Sema &S, Decl *D, Kind Kind) + : S(S), Key(D->getCanonicalDecl(), unsigned(Kind)) { + auto [_, Created] = S.InstantiatingSpecializations.insert(Key); + if (!Created) + Key = {}; + } + + ~RecursiveInstGuard() { + if (Key.getOpaqueValue()) { + [[maybe_unused]] bool Found = S.InstantiatingSpecializations.erase(Key); + assert(Found); + } + } + + operator bool() const { return Key.getOpaqueValue() == nullptr; } + + private: + Sema &S; + Sema::InstantiatingSpecializationsKey Key; + }; + /// A context in which code is being synthesized (where a source location /// alone is not sufficient to identify the context). This covers template /// instantiation and various forms of implicitly-generated functions. @@ -13356,14 +13386,9 @@ class Sema final : public SemaBase { /// recursive template instantiations. bool isInvalid() const { return Invalid; } - /// Determine whether we are already instantiating this - /// specialization in some surrounding active instantiation. - bool isAlreadyInstantiating() const { return AlreadyInstantiating; } - private: Sema &SemaRef; bool Invalid; - bool AlreadyInstantiating; InstantiatingTemplate(Sema &SemaRef, CodeSynthesisContext::SynthesisKind Kind, @@ -13486,7 +13511,7 @@ class Sema final : public SemaBase { SmallVector CodeSynthesisContexts; /// Specializations whose definitions are currently being instantiated. - llvm::DenseSet> InstantiatingSpecializations; + llvm::DenseSet InstantiatingSpecializations; /// Non-dependent types used in templates that have already been instantiated /// by some template instantiation. diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 7b05e4cf1385f..69b76378db3bc 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -638,15 +638,8 @@ Sema::InstantiatingTemplate::InstantiatingTemplate( } Invalid = SemaRef.pushCodeSynthesisContext(Inst); - if (!Invalid) { - AlreadyInstantiating = - !Inst.Entity - ? false - : !SemaRef.InstantiatingSpecializations - .insert({Inst.Entity->getCanonicalDecl(), Inst.Kind}) - .second; + if (!Invalid) atTemplateBegin(SemaRef.TemplateInstCallbacks, SemaRef, Inst); - } } Sema::InstantiatingTemplate::InstantiatingTemplate( @@ -901,13 +894,6 @@ void Sema::popCodeSynthesisContext() { void Sema::InstantiatingTemplate::Clear() { if (!Invalid) { - if (!AlreadyInstantiating) { - auto &Active = SemaRef.CodeSynthesisContexts.back(); - if (Active.Entity) - SemaRef.InstantiatingSpecializations.erase( - {Active.Entity->getCanonicalDecl(), Active.Kind}); - } - atTemplateEnd(SemaRef.TemplateInstCallbacks, SemaRef, SemaRef.CodeSynthesisContexts.back()); @@ -3311,17 +3297,21 @@ bool Sema::SubstDefaultArgument( FunctionDecl *FD = cast(Param->getDeclContext()); Expr *PatternExpr = Param->getUninstantiatedDefaultArg(); + RecursiveInstGuard AlreadyInstantiating( + *this, Param, RecursiveInstGuard::Kind::DefaultArgument); + if (AlreadyInstantiating) { + Diag(Param->getBeginLoc(), diag::err_recursive_default_argument) + << FD << PatternExpr->getSourceRange(); + Param->setInvalidDecl(); + return true; + } + EnterExpressionEvaluationContext EvalContext( *this, ExpressionEvaluationContext::PotentiallyEvaluated, Param); InstantiatingTemplate Inst(*this, Loc, Param, TemplateArgs.getInnermost()); if (Inst.isInvalid()) return true; - if (Inst.isAlreadyInstantiating()) { - Diag(Param->getBeginLoc(), diag::err_recursive_default_argument) << FD; - Param->setInvalidDecl(); - return true; - } ExprResult Result; // C++ [dcl.fct.default]p5: @@ -3559,6 +3549,12 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation, const MultiLevelTemplateArgumentList &TemplateArgs, TemplateSpecializationKind TSK, bool Complain) { +#ifndef NDEBUG + RecursiveInstGuard AlreadyInstantiating(*this, Instantiation, + RecursiveInstGuard::Kind::Template); + assert(!AlreadyInstantiating && "should have been caught by caller"); +#endif + CXXRecordDecl *PatternDef = cast_or_null(Pattern->getDefinition()); if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Instantiation, @@ -3595,7 +3591,6 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation, InstantiatingTemplate Inst(*this, PointOfInstantiation, Instantiation); if (Inst.isInvalid()) return true; - assert(!Inst.isAlreadyInstantiating() && "should have been caught by caller"); PrettyDeclStackTraceEntry CrashInfo(Context, Instantiation, SourceLocation(), "instantiating class definition"); @@ -3807,6 +3802,12 @@ bool Sema::InstantiateEnum(SourceLocation PointOfInstantiation, EnumDecl *Instantiation, EnumDecl *Pattern, const MultiLevelTemplateArgumentList &TemplateArgs, TemplateSpecializationKind TSK) { +#ifndef NDEBUG + RecursiveInstGuard AlreadyInstantiating(*this, Instantiation, + RecursiveInstGuard::Kind::Template); + assert(!AlreadyInstantiating && "should have been caught by caller"); +#endif + EnumDecl *PatternDef = Pattern->getDefinition(); if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Instantiation, Instantiation->getInstantiatedFromMemberEnum(), @@ -3824,8 +3825,6 @@ bool Sema::InstantiateEnum(SourceLocation PointOfInstantiation, InstantiatingTemplate Inst(*this, PointOfInstantiation, Instantiation); if (Inst.isInvalid()) return true; - if (Inst.isAlreadyInstantiating()) - return false; PrettyDeclStackTraceEntry CrashInfo(Context, Instantiation, SourceLocation(), "instantiating enum definition"); @@ -3864,6 +3863,15 @@ bool Sema::InstantiateInClassInitializer( Pattern->getInClassInitStyle() && "pattern and instantiation disagree about init style"); + RecursiveInstGuard AlreadyInstantiating(*this, Instantiation, + RecursiveInstGuard::Kind::Template); + if (AlreadyInstantiating) { + // Error out if we hit an instantiation cycle for this initializer. + Diag(PointOfInstantiation, diag::err_default_member_initializer_cycle) + << Instantiation; + return true; + } + // Error out if we haven't parsed the initializer of the pattern yet because // we are waiting for the closing brace of the outer class. Expr *OldInit = Pattern->getInClassInitializer(); @@ -3882,12 +3890,6 @@ bool Sema::InstantiateInClassInitializer( InstantiatingTemplate Inst(*this, PointOfInstantiation, Instantiation); if (Inst.isInvalid()) return true; - if (Inst.isAlreadyInstantiating()) { - // Error out if we hit an instantiation cycle for this initializer. - Diag(PointOfInstantiation, diag::err_default_member_initializer_cycle) - << Instantiation; - return true; - } PrettyDeclStackTraceEntry CrashInfo(Context, Instantiation, SourceLocation(), "instantiating default member init"); @@ -3968,11 +3970,15 @@ static ActionResult getPatternForClassTemplateSpecialization( Sema &S, SourceLocation PointOfInstantiation, ClassTemplateSpecializationDecl *ClassTemplateSpec, TemplateSpecializationKind TSK, bool PrimaryStrictPackMatch) { +#ifndef NDEBUG + Sema::RecursiveInstGuard AlreadyInstantiating( + S, ClassTemplateSpec, Sema::RecursiveInstGuard::Kind::Template); + assert(!AlreadyInstantiating && "should have been caught by caller"); +#endif + Sema::InstantiatingTemplate Inst(S, PointOfInstantiation, ClassTemplateSpec); if (Inst.isInvalid()) return {/*Invalid=*/true}; - if (Inst.isAlreadyInstantiating()) - return {/*Invalid=*/false}; llvm::PointerUnion diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 3819f775811e5..384ef3f3c6f34 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5282,6 +5282,16 @@ void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation, if (Proto->getExceptionSpecType() != EST_Uninstantiated) return; + RecursiveInstGuard AlreadyInstantiating( + *this, Decl, RecursiveInstGuard::Kind::ExceptionSpec); + if (AlreadyInstantiating) { + // This exception specification indirectly depends on itself. Reject. + // FIXME: Corresponding rule in the standard? + Diag(PointOfInstantiation, diag::err_exception_spec_cycle) << Decl; + UpdateExceptionSpec(Decl, EST_None); + return; + } + InstantiatingTemplate Inst(*this, PointOfInstantiation, Decl, InstantiatingTemplate::ExceptionSpecification()); if (Inst.isInvalid()) { @@ -5290,13 +5300,6 @@ void Sema::InstantiateExceptionSpec(SourceLocation PointOfInstantiation, UpdateExceptionSpec(Decl, EST_None); return; } - if (Inst.isAlreadyInstantiating()) { - // This exception specification indirectly depends on itself. Reject. - // FIXME: Corresponding rule in the standard? - Diag(PointOfInstantiation, diag::err_exception_spec_cycle) << Decl; - UpdateExceptionSpec(Decl, EST_None); - return; - } // Enter the scope of this instantiation. We don't use // PushDeclContext because we don't have a scope. @@ -5356,8 +5359,6 @@ TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New, if (ActiveInst.Kind == ActiveInstType::ExplicitTemplateArgumentSubstitution || ActiveInst.Kind == ActiveInstType::DeducedTemplateArgumentSubstitution) { if (isa(ActiveInst.Entity)) { - SemaRef.InstantiatingSpecializations.erase( - {ActiveInst.Entity->getCanonicalDecl(), ActiveInst.Kind}); atTemplateEnd(SemaRef.TemplateInstCallbacks, SemaRef, ActiveInst); ActiveInst.Kind = ActiveInstType::TemplateInstantiation; ActiveInst.Entity = New; @@ -5515,6 +5516,12 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, Function = const_cast(ExistingDefn); } +#ifndef NDEBUG + RecursiveInstGuard AlreadyInstantiating(*this, Function, + RecursiveInstGuard::Kind::Template); + assert(!AlreadyInstantiating && "should have been caught by caller"); +#endif + // Find the function body that we'll be substituting. const FunctionDecl *PatternDecl = Function->getTemplateInstantiationPattern(); assert(PatternDecl && "instantiating a non-template"); @@ -5654,7 +5661,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, } InstantiatingTemplate Inst(*this, PointOfInstantiation, Function); - if (Inst.isInvalid() || Inst.isAlreadyInstantiating()) + if (Inst.isInvalid()) return; PrettyDeclStackTraceEntry CrashInfo(Context, Function, SourceLocation(), "instantiating function definition"); @@ -6223,6 +6230,11 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, if (TSK == TSK_ExplicitSpecialization) return; + RecursiveInstGuard AlreadyInstantiating(*this, Var, + RecursiveInstGuard::Kind::Template); + if (AlreadyInstantiating) + return; + // Find the pattern and the arguments to substitute into it. VarDecl *PatternDecl = Var->getTemplateInstantiationPattern(); assert(PatternDecl && "no pattern for templated variable"); @@ -6246,7 +6258,7 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, // FIXME: Factor out the duplicated instantiation context setup/tear down // code here. InstantiatingTemplate Inst(*this, PointOfInstantiation, Var); - if (Inst.isInvalid() || Inst.isAlreadyInstantiating()) + if (Inst.isInvalid()) return; PrettyDeclStackTraceEntry CrashInfo(Context, Var, SourceLocation(), "instantiating variable initializer"); @@ -6350,7 +6362,7 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation, } InstantiatingTemplate Inst(*this, PointOfInstantiation, Var); - if (Inst.isInvalid() || Inst.isAlreadyInstantiating()) + if (Inst.isInvalid()) return; PrettyDeclStackTraceEntry CrashInfo(Context, Var, SourceLocation(), "instantiating variable definition"); diff --git a/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp b/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp index 6b8ca4f740914..a4775710e6d8c 100644 --- a/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp +++ b/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp @@ -82,13 +82,14 @@ namespace sad { template void swap(T &, T &); template struct CLASS { - void swap(CLASS &other) noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}} expected-note {{declared here}} - // expected-error@-1{{uses itself}} expected-note@-1{{in instantiation of}} + void swap(CLASS &other) // expected-note {{declared here}} + noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}} + // expected-error@-1{{uses itself}} }; CLASS pi; - static_assert(!noexcept(pi.swap(pi)), ""); // expected-note 2{{in instantiation of exception specification for 'swap'}} + static_assert(!noexcept(pi.swap(pi)), ""); // expected-note {{in instantiation of exception specification for 'swap'}} } #endif diff --git a/clang/test/SemaTemplate/instantiate-self.cpp b/clang/test/SemaTemplate/instantiate-self.cpp index 4999a4ad3784d..a99a2ef30b479 100644 --- a/clang/test/SemaTemplate/instantiate-self.cpp +++ b/clang/test/SemaTemplate/instantiate-self.cpp @@ -86,7 +86,7 @@ namespace test7 { namespace test8 { template struct A { - int n = A{}.n; // expected-error {{default member initializer for 'n' uses itself}} expected-note {{instantiation of default member init}} + int n = A{}.n; // expected-error {{default member initializer for 'n' uses itself}} }; A ai = {}; // expected-note {{instantiation of default member init}} } @@ -100,7 +100,7 @@ namespace test9 { namespace test10 { template struct A { - void f() noexcept(noexcept(f())); // expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}} + void f() noexcept(noexcept(f())); // expected-error {{exception specification of 'f' uses itself}} }; bool b = noexcept(A().f()); // expected-note {{instantiation of}} } @@ -125,7 +125,7 @@ namespace test11 { } namespace test12 { - template int f(T t, int = f(T())) {} // expected-error {{recursive evaluation of default argument}} expected-note {{instantiation of}} + template int f(T t, int = f(T())) {} // expected-error {{recursive evaluation of default argument}} struct X {}; int q = f(X()); // expected-note {{instantiation of}} }