[clang] Revert "[clang] separate recursive instantiation check from CodeSynthesisContext" (PR #164174)
Matheus Izvekov via cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 19 12:21:26 PDT 2025
https://github.com/mizvekov created https://github.com/llvm/llvm-project/pull/164174
Reverts llvm/llvm-project#162224
Broke buildbot here: https://github.com/llvm/llvm-project/pull/162224#issuecomment-3419843479
>From 2b5c693610d5e52663e534ab7af9d30fdac3c2e0 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov <mizvekov at gmail.com>
Date: Sun, 19 Oct 2025 16:19:06 -0300
Subject: [PATCH] =?UTF-8?q?Revert=20"[clang]=20separate=20recursive=20inst?=
=?UTF-8?q?antiation=20check=20from=20CodeSynthesisCont=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit fd073a3fbaf0b05fae61cca5def80ce0adaeadb3.
---
clang/include/clang/Sema/Sema.h | 46 ++--------
clang/lib/Sema/SemaTemplateInstantiate.cpp | 87 ++++++++-----------
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 36 +++-----
.../test/SemaCXX/libstdcxx_pair_swap_hack.cpp | 7 +-
clang/test/SemaTemplate/instantiate-self.cpp | 28 +-----
5 files changed, 62 insertions(+), 142 deletions(-)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cb21335ede075..add4c1596ab86 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13007,37 +13007,6 @@ 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.
@@ -13399,9 +13368,14 @@ 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,
@@ -13531,7 +13505,7 @@ class Sema final : public SemaBase {
SmallVector<CodeSynthesisContext, 16> CodeSynthesisContexts;
/// Specializations whose definitions are currently being instantiated.
- llvm::DenseSet<InstantiatingSpecializationsKey> InstantiatingSpecializations;
+ llvm::DenseSet<std::pair<Decl *, unsigned>> InstantiatingSpecializations;
/// Non-dependent types used in templates that have already been instantiated
/// by some template instantiation.
@@ -13806,14 +13780,6 @@ 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 7f858050db13e..038f39633760d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -639,8 +639,15 @@ Sema::InstantiatingTemplate::InstantiatingTemplate(
}
Invalid = SemaRef.pushCodeSynthesisContext(Inst);
- if (!Invalid)
+ if (!Invalid) {
+ AlreadyInstantiating =
+ !Inst.Entity
+ ? false
+ : !SemaRef.InstantiatingSpecializations
+ .insert({Inst.Entity->getCanonicalDecl(), Inst.Kind})
+ .second;
atTemplateBegin(SemaRef.TemplateInstCallbacks, SemaRef, Inst);
+ }
}
Sema::InstantiatingTemplate::InstantiatingTemplate(
@@ -895,6 +902,13 @@ 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());
@@ -3298,20 +3312,17 @@ 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:
@@ -3543,26 +3554,12 @@ namespace clang {
}
}
-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) {
-
+bool
+Sema::InstantiateClass(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,
@@ -3599,6 +3596,7 @@ bool Sema::InstantiateClassImpl(
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");
@@ -3810,12 +3808,6 @@ 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(),
@@ -3833,6 +3825,8 @@ 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");
@@ -3871,14 +3865,6 @@ 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();
@@ -3897,6 +3883,12 @@ 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");
@@ -3980,6 +3972,8 @@ 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 *>
@@ -4142,11 +4136,6 @@ 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;
@@ -4159,7 +4148,7 @@ bool Sema::InstantiateClassTemplateSpecialization(
if (!Pattern.isUsable())
return Pattern.isInvalid();
- bool Err = InstantiateClassImpl(
+ bool Err = InstantiateClass(
PointOfInstantiation, ClassTemplateSpec, Pattern.get(),
getTemplateInstantiationArgs(ClassTemplateSpec), TSK, Complain);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 28925cca8f956..4863b4522a92d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5312,16 +5312,6 @@ 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()) {
@@ -5330,6 +5320,13 @@ 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.
@@ -5389,6 +5386,8 @@ 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;
@@ -5546,12 +5545,6 @@ 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");
@@ -5691,7 +5684,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
}
InstantiatingTemplate Inst(*this, PointOfInstantiation, Function);
- if (Inst.isInvalid())
+ if (Inst.isInvalid() || Inst.isAlreadyInstantiating())
return;
PrettyDeclStackTraceEntry CrashInfo(Context, Function, SourceLocation(),
"instantiating function definition");
@@ -6260,11 +6253,6 @@ 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");
@@ -6288,7 +6276,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())
+ if (Inst.isInvalid() || Inst.isAlreadyInstantiating())
return;
PrettyDeclStackTraceEntry CrashInfo(Context, Var, SourceLocation(),
"instantiating variable initializer");
@@ -6392,7 +6380,7 @@ void Sema::InstantiateVariableDefinition(SourceLocation PointOfInstantiation,
}
InstantiatingTemplate Inst(*this, PointOfInstantiation, Var);
- if (Inst.isInvalid())
+ if (Inst.isInvalid() || Inst.isAlreadyInstantiating())
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 a4775710e6d8c..6b8ca4f740914 100644
--- a/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
+++ b/clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
@@ -82,14 +82,13 @@ namespace sad {
template<typename T> void swap(T &, T &);
template<typename A, typename B> struct CLASS {
- void swap(CLASS &other) // expected-note {{declared here}}
- noexcept(noexcept(swap(*this, other))); // expected-error {{too many arguments}}
- // expected-error at -1{{uses itself}}
+ 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}}
};
CLASS<int, int> pi;
- static_assert(!noexcept(pi.swap(pi)), ""); // expected-note {{in instantiation of exception specification for 'swap'}}
+ static_assert(!noexcept(pi.swap(pi)), ""); // expected-note 2{{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 1cdf2c6dd503e..4999a4ad3784d 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}}
+ int n = A{}.n; // expected-error {{default member initializer for 'n' uses itself}} expected-note {{instantiation of default member init}}
};
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}}
+ void f() noexcept(noexcept(f())); // expected-error {{exception specification of 'f' uses itself}} expected-note {{instantiation of}}
};
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}}
+ template<typename T> int f(T t, int = f(T())) {} // expected-error {{recursive evaluation of default argument}} expected-note {{instantiation of}}
struct X {};
int q = f(X()); // expected-note {{instantiation of}}
}
@@ -171,25 +171,3 @@ 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