Revert "[clang] separate recursive instantiation check from CodeSynthesisContext" #164174
+62
−142
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…esisCont…" This reverts commit fd073a3.
llvmbot
added
clang
mizvekov
deleted the
revert-162224-users/mizvekov/refactor-recursive-inst-diag
branch
|
@llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) ChangesReverts llvm/llvm-project#162224 Broke buildbot here: #162224 (comment) Full diff: https://github.com/llvm/llvm-project/pull/164174.diff 5 Files Affected:
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@-1{{uses itself}}
+ 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}}
};
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
|
mizvekov
added
the
skip-precommit-approval
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reverts #162224
Broke buildbot here: #162224 (comment)