Revert "[clang] separate recursive instantiation check from CodeSynthesisContext" (#164174)

mizvekov · web-flow · commit 8091dcea7fde · 2025-10-19T16:21:47.000-03:00
Reverts #162224 Broke buildbot here: #162224 (comment)
diff --git 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
@@ -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
@@ -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
@@ -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
