[clang] 63ca2fd - [clang] Reland: separate recursive instantiation check from CodeSynthesisContext (#164177)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 19 12:45:54 PDT 2025
Author: Matheus Izvekov
Date: 2025-10-19T16:45:50-03:00
New Revision: 63ca2fd7a16f532a95e53780220d2eae0debb8d9
URL: https://github.com/llvm/llvm-project/commit/63ca2fd7a16f532a95e53780220d2eae0debb8d9
DIFF: https://github.com/llvm/llvm-project/commit/63ca2fd7a16f532a95e53780220d2eae0debb8d9.diff
LOG: [clang] Reland: separate recursive instantiation check from CodeSynthesisContext (#164177)
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.
This relands #162224 with no changes, turns out the buildbot failure was
unrelated.
Added:
Modified:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
clang/test/SemaTemplate/instantiate-self.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index add4c1596ab86..cb21335ede075 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13007,6 +13007,37 @@ class Sema final : public SemaBase {
/// default arguments of its methods have been parsed.
UnparsedDefaultArgInstantiationsMap UnparsedDefaultArgInstantiations;
+ using InstantiatingSpecializationsKey = llvm::PointerIntPair<Decl *, 2>;
+
+ 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 Erased =
+ S.InstantiatingSpecializations.erase(Key);
+ assert(Erased);
+ }
+ }
+
+ 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.
@@ -13368,14 +13399,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,
@@ -13505,7 +13531,7 @@ class Sema final : public SemaBase {
SmallVector<CodeSynthesisContext, 16> CodeSynthesisContexts;
/// Specializations whose definitions are currently being instantiated.
- llvm::DenseSet<std::pair<Decl *, unsigned>> InstantiatingSpecializations;
+ llvm::DenseSet<InstantiatingSpecializationsKey> InstantiatingSpecializations;
/// Non-dependent types used in templates that have already been instantiated
/// by some template instantiation.
@@ -13780,6 +13806,14 @@ class Sema final : public SemaBase {
const MultiLevelTemplateArgumentList &TemplateArgs,
TemplateSpecializationKind TSK, bool Complain = true);
+private:
+ bool InstantiateClassImpl(SourceLocation PointOfInstantiation,
+ CXXRecordDecl *Instantiation,
+ CXXRecordDecl *Pattern,
+ const MultiLevelTemplateArgumentList &TemplateArgs,
+ TemplateSpecializationKind TSK, bool Complain);
+
+public:
/// Instantiate the definition of an enum from a given pattern.
///
/// \param PointOfInstantiation The point of instantiation within the
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 038f39633760d..7f858050db13e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -639,15 +639,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(
@@ -902,13 +895,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());
@@ -3312,17 +3298,20 @@ bool Sema::SubstDefaultArgument(
FunctionDecl *FD = cast<FunctionDecl>(Param->getDeclContext());
Expr *PatternExpr = Param->getUninstantiatedDefaultArg();
+ RecursiveInstGuard AlreadyInstantiating(
+ *this, Param, RecursiveInstGuard::Kind::DefaultArgument);
+ if (AlreadyInstantiating) {
+ Param->setInvalidDecl();
+ return Diag(Param->getBeginLoc(), diag::err_recursive_default_argument)
+ << FD << PatternExpr->getSourceRange();
+ }
+
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:
@@ -3554,12 +3543,26 @@ namespace clang {
}
}
-bool
-Sema::InstantiateClass(SourceLocation PointOfInstantiation,
- CXXRecordDecl *Instantiation, CXXRecordDecl *Pattern,
- const MultiLevelTemplateArgumentList &TemplateArgs,
- TemplateSpecializationKind TSK,
- bool Complain) {
+bool Sema::InstantiateClass(SourceLocation PointOfInstantiation,
+ CXXRecordDecl *Instantiation,
+ CXXRecordDecl *Pattern,
+ 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
+
+ return InstantiateClassImpl(PointOfInstantiation, Instantiation, Pattern,
+ TemplateArgs, TSK, Complain);
+}
+
+bool Sema::InstantiateClassImpl(
+ SourceLocation PointOfInstantiation, CXXRecordDecl *Instantiation,
+ CXXRecordDecl *Pattern, const MultiLevelTemplateArgumentList &TemplateArgs,
+ TemplateSpecializationKind TSK, bool Complain) {
+
CXXRecordDecl *PatternDef
= cast_or_null<CXXRecordDecl>(Pattern->getDefinition());
if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Instantiation,
@@ -3596,7 +3599,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");
@@ -3808,6 +3810,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(),
@@ -3825,8 +3833,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");
@@ -3865,6 +3871,14 @@ 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.
+ return Diag(PointOfInstantiation,
+ diag::err_default_member_initializer_cycle)
+ << Instantiation;
+
// 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();
@@ -3883,12 +3897,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");
@@ -3972,8 +3980,6 @@ static ActionResult<CXXRecordDecl *> getPatternForClassTemplateSpecialization(
Sema::InstantiatingTemplate Inst(S, PointOfInstantiation, ClassTemplateSpec);
if (Inst.isInvalid())
return {/*Invalid=*/true};
- if (Inst.isAlreadyInstantiating())
- return {/*Invalid=*/false};
llvm::PointerUnion<ClassTemplateDecl *,
ClassTemplatePartialSpecializationDecl *>
@@ -4136,6 +4142,11 @@ bool Sema::InstantiateClassTemplateSpecialization(
if (ClassTemplateSpec->isInvalidDecl())
return true;
+ Sema::RecursiveInstGuard AlreadyInstantiating(
+ *this, ClassTemplateSpec, Sema::RecursiveInstGuard::Kind::Template);
+ if (AlreadyInstantiating)
+ return false;
+
bool HadAvaibilityWarning =
ShouldDiagnoseAvailabilityOfDecl(ClassTemplateSpec, nullptr, nullptr)
.first != AR_Available;
@@ -4148,7 +4159,7 @@ bool Sema::InstantiateClassTemplateSpecialization(
if (!Pattern.isUsable())
return Pattern.isInvalid();
- bool Err = InstantiateClass(
+ bool Err = InstantiateClassImpl(
PointOfInstantiation, ClassTemplateSpec, Pattern.get(),
getTemplateInstantiationArgs(ClassTemplateSpec), TSK, Complain);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 4863b4522a92d..28925cca8f956 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5312,6 +5312,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()) {
@@ -5320,13 +5330,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.
@@ -5386,8 +5389,6 @@ TemplateDeclInstantiator::InitFunctionInstantiation(FunctionDecl *New,
if (ActiveInst.Kind == ActiveInstType::ExplicitTemplateArgumentSubstitution ||
ActiveInst.Kind == ActiveInstType::DeducedTemplateArgumentSubstitution) {
if (isa<FunctionTemplateDecl>(ActiveInst.Entity)) {
- SemaRef.InstantiatingSpecializations.erase(
- {ActiveInst.Entity->getCanonicalDecl(), ActiveInst.Kind});
atTemplateEnd(SemaRef.TemplateInstCallbacks, SemaRef, ActiveInst);
ActiveInst.Kind = ActiveInstType::TemplateInstantiation;
ActiveInst.Entity = New;
@@ -5545,6 +5546,12 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function = const_cast<FunctionDecl*>(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");
@@ -5684,7 +5691,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");
@@ -6253,6 +6260,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");
@@ -6276,7 +6288,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");
@@ -6380,7 +6392,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<typename T> void swap(T &, T &);
template<typename A, typename B> struct CLASS {
- void swap(CLASS &other) noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}} expected-note {{declared here}}
- // expected-error at -1{{uses itself}} expected-note at -1{{in instantiation of}}
+ void swap(CLASS &other) // expected-note {{declared here}}
+ noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}}
+ // expected-error at -1{{uses itself}}
};
CLASS<int, int> 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..1cdf2c6dd503e 100644
--- a/clang/test/SemaTemplate/instantiate-self.cpp
+++ b/clang/test/SemaTemplate/instantiate-self.cpp
@@ -86,7 +86,7 @@ namespace test7 {
namespace test8 {
template<typename T> 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<int> ai = {}; // expected-note {{instantiation of default member init}}
}
@@ -100,7 +100,7 @@ namespace test9 {
namespace test10 {
template<typename T> 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<int>().f()); // expected-note {{instantiation of}}
}
@@ -125,7 +125,7 @@ namespace test11 {
}
namespace test12 {
- template<typename T> int f(T t, int = f(T())) {} // expected-error {{recursive evaluation of default argument}} expected-note {{instantiation of}}
+ template<typename T> int f(T t, int = f(T())) {} // expected-error {{recursive evaluation of default argument}}
struct X {};
int q = f(X()); // expected-note {{instantiation of}}
}
@@ -171,3 +171,25 @@ namespace test13 {
A::Z<A> aza;
#endif
}
+
+namespace test14 {
+ template <class> void f();
+ template <class T, decltype(new (f<void>()) T)> T x;
+}
+
+namespace test15 {
+ template <class V> void __overload(V);
+
+ template <class, class> struct __invoke_result_impl;
+ template <class _Arg>
+ struct __invoke_result_impl<decltype(__overload(*(_Arg*)0)),
+ _Arg>;
+ struct variant {
+ template <class _Arg,
+ class = typename __invoke_result_impl<void, _Arg>::type>
+ variant(_Arg);
+ };
+ struct Matcher {
+ Matcher(variant);
+ } vec(vec); // expected-warning {{uninitialized}}
+} // namespace test15
More information about the cfe-commits
mailing list