[Clang] Reapply "Only remove lambda scope after computing evaluation context"

[zyn0217]

The immediate evaluation context needs the lambda scope info to propagate some flags, however that LSI was removed in ActOnFinishFunctionBody which happened before rebuilding a lambda expression.

The last attempt destroyed LSI at the end of the block scope, after which we still need it in DiagnoseShadowingLambdaDecls.

This also converts the wrapper function to default arguments as a drive-by fix, as well as does some cleanup.

Fixes #145776

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

The immediate evaluation context needs the lambda scope info to propagate some flags, however that LSI was removed in ActOnFinishFunctionBody which happened before rebuilding a lambda expression.

This also converts the wrapper function to default arguments as a drive-by fix.

The last attempt destroyed LSI at the end of the block scope, after which we still need it in DiagnoseShadowingLambdaDecls.

Fixes #145776

---

Full diff: https://github.com/llvm/llvm-project/pull/154458.diff

6 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/include/clang/Sema/Sema.h (+14-8)
• (modified) clang/lib/Sema/SemaDecl.cpp (+4-7)
• (modified) clang/lib/Sema/SemaLambda.cpp (+14-5)
• (modified) clang/lib/Sema/TreeTransform.h (+5-8)
• (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+16)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9231f2cae9bfa..ca6fc663c9525 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -158,6 +158,7 @@ Bug Fixes to C++ Support
 - Diagnose binding a reference to ``*nullptr`` during constant evaluation. (#GH48665)
 - Suppress ``-Wdeprecated-declarations`` in implicitly generated functions. (#GH147293)
 - Fix a crash when deleting a pointer to an incomplete array (#GH150359).
+- Fixed a mismatched lambda scope bug when propagating up ``consteval`` within nested lambdas. (#GH145776)
 - Fix an assertion failure when expression in assumption attribute
   (``[[assume(expr)]]``) creates temporary objects.
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5211373367677..b63c108ab47d8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4179,8 +4179,15 @@ class Sema final : public SemaBase {
   /// return statement in the scope of a variable has the same NRVO candidate,
   /// that candidate is an NRVO variable.
   void computeNRVO(Stmt *Body, sema::FunctionScopeInfo *Scope);
-  Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
-  Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
+
+  /// Performs semantic analysis at the end of a function body.
+  ///
+  /// \param RetainFunctionScopeInfo If \c true, the client is responsible for
+  /// releasing the associated \p FunctionScopeInfo. This is useful when
+  /// building e.g. LambdaExprs.
+  Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body,
+                                bool IsInstantiation = false,
+                                bool RetainFunctionScopeInfo = false);
   Decl *ActOnSkippedFunctionBody(Decl *Decl);
   void ActOnFinishInlineFunctionDef(FunctionDecl *D);
 
@@ -6873,23 +6880,23 @@ class Sema final : public SemaBase {
     assert(!ExprEvalContexts.empty() &&
            "Must be in an expression evaluation context");
     return ExprEvalContexts.back();
-  };
+  }
 
   ExpressionEvaluationContextRecord &currentEvaluationContext() {
     assert(!ExprEvalContexts.empty() &&
            "Must be in an expression evaluation context");
     return ExprEvalContexts.back();
-  };
+  }
 
   ExpressionEvaluationContextRecord &parentEvaluationContext() {
     assert(ExprEvalContexts.size() >= 2 &&
            "Must be in an expression evaluation context");
     return ExprEvalContexts[ExprEvalContexts.size() - 2];
-  };
+  }
 
   const ExpressionEvaluationContextRecord &parentEvaluationContext() const {
     return const_cast<Sema *>(this)->parentEvaluationContext();
-  };
+  }
 
   bool isAttrContext() const {
     return ExprEvalContexts.back().ExprContext ==
@@ -9139,8 +9146,7 @@ class Sema final : public SemaBase {
 
   /// Complete a lambda-expression having processed and attached the
   /// lambda body.
-  ExprResult BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
-                             sema::LambdaScopeInfo *LSI);
+  ExprResult BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc);
 
   /// Get the return type to use for a lambda's conversion function(s) to
   /// function pointer type, given the type of the call operator.
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b5eb825eb52cc..4c8974b2e6c9a 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16149,10 +16149,6 @@ Decl *Sema::ActOnSkippedFunctionBody(Decl *Decl) {
   return Decl;
 }
 
-Decl *Sema::ActOnFinishFunctionBody(Decl *D, Stmt *BodyArg) {
-  return ActOnFinishFunctionBody(D, BodyArg, /*IsInstantiation=*/false);
-}
-
 /// RAII object that pops an ExpressionEvaluationContext when exiting a function
 /// body.
 class ExitFunctionBodyRAII {
@@ -16223,8 +16219,8 @@ void Sema::CheckCoroutineWrapper(FunctionDecl *FD) {
     Diag(FD->getLocation(), diag::err_coroutine_return_type) << RD;
 }
 
-Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
-                                    bool IsInstantiation) {
+Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, bool IsInstantiation,
+                                    bool RetainFunctionScopeInfo) {
   FunctionScopeInfo *FSI = getCurFunction();
   FunctionDecl *FD = dcl ? dcl->getAsFunction() : nullptr;
 
@@ -16681,7 +16677,8 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body,
   if (!IsInstantiation)
     PopDeclContext();
 
-  PopFunctionScopeInfo(ActivePolicy, dcl);
+  if (!RetainFunctionScopeInfo)
+    PopFunctionScopeInfo(ActivePolicy, dcl);
   // If any errors have occurred, clear out any temporaries that may have
   // been leftover. This ensures that these temporaries won't be picked up for
   // deletion in some later function.
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index bc3c4b0addeba..30df92a4b1514 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1968,14 +1968,15 @@ ExprResult Sema::BuildCaptureInit(const Capture &Cap,
 }
 
 ExprResult Sema::ActOnLambdaExpr(SourceLocation StartLoc, Stmt *Body) {
-  LambdaScopeInfo LSI = *cast<LambdaScopeInfo>(FunctionScopes.back());
+  LambdaScopeInfo &LSI = *cast<LambdaScopeInfo>(FunctionScopes.back());
 
   if (LSI.CallOperator->hasAttr<SYCLKernelEntryPointAttr>())
     SYCL().CheckSYCLEntryPointFunctionDecl(LSI.CallOperator);
 
-  ActOnFinishFunctionBody(LSI.CallOperator, Body);
+  ActOnFinishFunctionBody(LSI.CallOperator, Body, /*IsInstantiation=*/false,
+                          /*RetainFunctionScopeInfo=*/true);
 
-  return BuildLambdaExpr(StartLoc, Body->getEndLoc(), &LSI);
+  return BuildLambdaExpr(StartLoc, Body->getEndLoc());
 }
 
 static LambdaCaptureDefault
@@ -2132,8 +2133,9 @@ ConstructFixItRangeForUnusedCapture(Sema &S, SourceRange CaptureRange,
   return SourceRange(FixItStart, FixItEnd);
 }
 
-ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
-                                 LambdaScopeInfo *LSI) {
+ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc,
+                                 SourceLocation EndLoc) {
+  LambdaScopeInfo *LSI = cast<LambdaScopeInfo>(FunctionScopes.back());
   // Collect information from the lambda scope.
   SmallVector<LambdaCapture, 4> Captures;
   SmallVector<Expr *, 4> CaptureInits;
@@ -2148,6 +2150,7 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
   CleanupInfo LambdaCleanup;
   bool ContainsUnexpandedParameterPack;
   bool IsGenericLambda;
+  PoppedFunctionScopePtr KeepLSI(nullptr, PoppedFunctionScopeDeleter(this));
   {
     CallOperator = LSI->CallOperator;
     Class = LSI->Lambda;
@@ -2170,6 +2173,12 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
 
     PopExpressionEvaluationContext();
 
+    sema::AnalysisBasedWarnings::Policy WP =
+        AnalysisWarnings.getPolicyInEffectAt(EndLoc);
+    // We cannot release LSI until we finish computing captures, which
+    // requires the scope to be popped.
+    KeepLSI = PopFunctionScopeInfo(&WP, LSI->CallOperator);
+
     // True if the current capture has a used capture or default before it.
     bool CurHasPreviousCapture = CaptureDefault != LCD_None;
     SourceLocation PrevCaptureLoc = CurHasPreviousCapture ?
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 5ff5fbf52ae25..735b06bd02f1c 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4073,7 +4073,7 @@ class TreeTransform {
             PVD->getUninstantiatedDefaultArg()
                 ->containsUnexpandedParameterPack();
     }
-    return getSema().BuildLambdaExpr(StartLoc, EndLoc, LSI);
+    return getSema().BuildLambdaExpr(StartLoc, EndLoc);
   }
 
   /// Build an empty C++1z fold-expression with the given operator.
@@ -15791,12 +15791,9 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     return ExprError();
   }
 
-  // Copy the LSI before ActOnFinishFunctionBody removes it.
-  // FIXME: This is dumb. Store the lambda information somewhere that outlives
-  // the call operator.
-  auto LSICopy = *LSI;
   getSema().ActOnFinishFunctionBody(NewCallOperator, Body.get(),
-                                    /*IsInstantiation*/ true);
+                                    /*IsInstantiation=*/true,
+                                    /*RetainFunctionScopeInfo=*/true);
   SavedContext.pop();
 
   // Recompute the dependency of the lambda so that we can defer the lambda call
@@ -15832,7 +15829,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   // *after* the substitution in case we can't decide the dependency
   // so early, e.g. because we want to see if any of the *substituted*
   // parameters are dependent.
-  DependencyKind = getDerived().ComputeLambdaDependency(&LSICopy);
+  DependencyKind = getDerived().ComputeLambdaDependency(LSI);
   Class->setLambdaDependencyKind(DependencyKind);
   // Clean up the type cache created previously. Then, we re-create a type for
   // such Decl with the new DependencyKind.
@@ -15840,7 +15837,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   getSema().Context.getTypeDeclType(Class);
 
   return getDerived().RebuildLambdaExpr(E->getBeginLoc(),
-                                        Body.get()->getEndLoc(), &LSICopy);
+                                        Body.get()->getEndLoc(), LSI);
 }
 
 template<typename Derived>
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index c4cfd9398920a..6cf0e0251ab62 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -610,3 +610,19 @@ namespace GH135281 {
   void (*ff)() = f2<B>; // expected-note {{instantiation of function template specialization}}
 }
 #endif
+
+namespace GH145776 {
+
+void runtime_only() {}
+consteval void comptime_only() {}
+
+void fn() {
+  []() {
+    runtime_only();
+    []() {
+      &comptime_only;
+    }();
+  }();
+}
+
+}

[cor3ntin]

LGTM