[Clang] Reapply "Only remove lambda scope after computing evaluation context" (#154458)

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

Author: zyn0217

clang/docs/ReleaseNotes.rst

@@ -227,6 +227,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.
 - Fix the dynamic_cast to final class optimization to correctly handle

clang/include/clang/Sema/Sema.h

@@ -4176,8 +4176,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);
 
@@ -6874,23 +6881,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 ==
@@ -9140,8 +9147,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.

clang/lib/Sema/SemaDecl.cpp

@@ -16224,10 +16224,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 {
@@ -16298,8 +16294,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;
 
@@ -16756,7 +16752,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.

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,156 +2133,149 @@ 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;
   SourceLocation CaptureDefaultLoc = LSI->CaptureDefaultLoc;
   LambdaCaptureDefault CaptureDefault =
       mapImplicitCaptureStyle(LSI->ImpCaptureStyle);
-  CXXRecordDecl *Class;
-  CXXMethodDecl *CallOperator;
-  SourceRange IntroducerRange;
-  bool ExplicitParams;
-  bool ExplicitResultType;
-  CleanupInfo LambdaCleanup;
-  bool ContainsUnexpandedParameterPack;
-  bool IsGenericLambda;
-  {
-    CallOperator = LSI->CallOperator;
-    Class = LSI->Lambda;
-    IntroducerRange = LSI->IntroducerRange;
-    ExplicitParams = LSI->ExplicitParams;
-    ExplicitResultType = !LSI->HasImplicitReturnType;
-    LambdaCleanup = LSI->Cleanup;
-    ContainsUnexpandedParameterPack = LSI->ContainsUnexpandedParameterPack;
-    IsGenericLambda = Class->isGenericLambda();
-
-    CallOperator->setLexicalDeclContext(Class);
-    Decl *TemplateOrNonTemplateCallOperatorDecl =
-        CallOperator->getDescribedFunctionTemplate()
-        ? CallOperator->getDescribedFunctionTemplate()
-        : cast<Decl>(CallOperator);
-
-    // FIXME: Is this really the best choice? Keeping the lexical decl context
-    // set as CurContext seems more faithful to the source.
-    TemplateOrNonTemplateCallOperatorDecl->setLexicalDeclContext(Class);
-
-    PopExpressionEvaluationContext();
-
-    // True if the current capture has a used capture or default before it.
-    bool CurHasPreviousCapture = CaptureDefault != LCD_None;
-    SourceLocation PrevCaptureLoc = CurHasPreviousCapture ?
-        CaptureDefaultLoc : IntroducerRange.getBegin();
-
-    for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I) {
-      const Capture &From = LSI->Captures[I];
-
-      if (From.isInvalid())
-        return ExprError();
-
-      assert(!From.isBlockCapture() && "Cannot capture __block variables");
-      bool IsImplicit = I >= LSI->NumExplicitCaptures;
-      SourceLocation ImplicitCaptureLoc =
-          IsImplicit ? CaptureDefaultLoc : SourceLocation();
-
-      // Use source ranges of explicit captures for fixits where available.
-      SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I];
-
-      // Warn about unused explicit captures.
-      bool IsCaptureUsed = true;
-      if (!CurContext->isDependentContext() && !IsImplicit &&
-          !From.isODRUsed()) {
-        // Initialized captures that are non-ODR used may not be eliminated.
-        // FIXME: Where did the IsGenericLambda here come from?
-        bool NonODRUsedInitCapture =
-            IsGenericLambda && From.isNonODRUsed() && From.isInitCapture();
-        if (!NonODRUsedInitCapture) {
-          bool IsLast = (I + 1) == LSI->NumExplicitCaptures;
-          SourceRange FixItRange = ConstructFixItRangeForUnusedCapture(
-              *this, CaptureRange, PrevCaptureLoc, CurHasPreviousCapture,
-              IsLast);
-          IsCaptureUsed =
-              !DiagnoseUnusedLambdaCapture(CaptureRange, FixItRange, From);
-        }
-      }
+  CXXRecordDecl *Class = LSI->Lambda;
+  CXXMethodDecl *CallOperator = LSI->CallOperator;
+  SourceRange IntroducerRange = LSI->IntroducerRange;
+  bool ExplicitParams = LSI->ExplicitParams;
+  bool ExplicitResultType = !LSI->HasImplicitReturnType;
+  CleanupInfo LambdaCleanup = LSI->Cleanup;
+  bool ContainsUnexpandedParameterPack = LSI->ContainsUnexpandedParameterPack;
+  bool IsGenericLambda = Class->isGenericLambda();
+
+  CallOperator->setLexicalDeclContext(Class);
+  Decl *TemplateOrNonTemplateCallOperatorDecl =
+      CallOperator->getDescribedFunctionTemplate()
+          ? CallOperator->getDescribedFunctionTemplate()
+          : cast<Decl>(CallOperator);
+
+  // FIXME: Is this really the best choice? Keeping the lexical decl context
+  // set as CurContext seems more faithful to the source.
+  TemplateOrNonTemplateCallOperatorDecl->setLexicalDeclContext(Class);
 
-      if (CaptureRange.isValid()) {
-        CurHasPreviousCapture |= IsCaptureUsed;
-        PrevCaptureLoc = CaptureRange.getEnd();
+  PopExpressionEvaluationContext();
+
+  sema::AnalysisBasedWarnings::Policy WP =
+      AnalysisWarnings.getPolicyInEffectAt(EndLoc);
+  // We cannot release LSI until we finish computing captures, which
+  // requires the scope to be popped.
+  Sema::PoppedFunctionScopePtr _ = 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 ? CaptureDefaultLoc : IntroducerRange.getBegin();
+
+  for (unsigned I = 0, N = LSI->Captures.size(); I != N; ++I) {
+    const Capture &From = LSI->Captures[I];
+
+    if (From.isInvalid())
+      return ExprError();
+
+    assert(!From.isBlockCapture() && "Cannot capture __block variables");
+    bool IsImplicit = I >= LSI->NumExplicitCaptures;
+    SourceLocation ImplicitCaptureLoc =
+        IsImplicit ? CaptureDefaultLoc : SourceLocation();
+
+    // Use source ranges of explicit captures for fixits where available.
+    SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I];
+
+    // Warn about unused explicit captures.
+    bool IsCaptureUsed = true;
+    if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) {
+      // Initialized captures that are non-ODR used may not be eliminated.
+      // FIXME: Where did the IsGenericLambda here come from?
+      bool NonODRUsedInitCapture =
+          IsGenericLambda && From.isNonODRUsed() && From.isInitCapture();
+      if (!NonODRUsedInitCapture) {
+        bool IsLast = (I + 1) == LSI->NumExplicitCaptures;
+        SourceRange FixItRange = ConstructFixItRangeForUnusedCapture(
+            *this, CaptureRange, PrevCaptureLoc, CurHasPreviousCapture, IsLast);
+        IsCaptureUsed =
+            !DiagnoseUnusedLambdaCapture(CaptureRange, FixItRange, From);
       }
+    }
 
-      // Map the capture to our AST representation.
-      LambdaCapture Capture = [&] {
-        if (From.isThisCapture()) {
-          // Capturing 'this' implicitly with a default of '[=]' is deprecated,
-          // because it results in a reference capture. Don't warn prior to
-          // C++2a; there's nothing that can be done about it before then.
-          if (getLangOpts().CPlusPlus20 && IsImplicit &&
-              CaptureDefault == LCD_ByCopy) {
-            Diag(From.getLocation(), diag::warn_deprecated_this_capture);
-            Diag(CaptureDefaultLoc, diag::note_deprecated_this_capture)
-                << FixItHint::CreateInsertion(
-                       getLocForEndOfToken(CaptureDefaultLoc), ", this");
-          }
-          return LambdaCapture(From.getLocation(), IsImplicit,
-                               From.isCopyCapture() ? LCK_StarThis : LCK_This);
-        } else if (From.isVLATypeCapture()) {
-          return LambdaCapture(From.getLocation(), IsImplicit, LCK_VLAType);
-        } else {
-          assert(From.isVariableCapture() && "unknown kind of capture");
-          ValueDecl *Var = From.getVariable();
-          LambdaCaptureKind Kind =
-              From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef;
-          return LambdaCapture(From.getLocation(), IsImplicit, Kind, Var,
-                               From.getEllipsisLoc());
-        }
-      }();
+    if (CaptureRange.isValid()) {
+      CurHasPreviousCapture |= IsCaptureUsed;
+      PrevCaptureLoc = CaptureRange.getEnd();
+    }
 
-      // Form the initializer for the capture field.
-      ExprResult Init = BuildCaptureInit(From, ImplicitCaptureLoc);
+    // Map the capture to our AST representation.
+    LambdaCapture Capture = [&] {
+      if (From.isThisCapture()) {
+        // Capturing 'this' implicitly with a default of '[=]' is deprecated,
+        // because it results in a reference capture. Don't warn prior to
+        // C++2a; there's nothing that can be done about it before then.
+        if (getLangOpts().CPlusPlus20 && IsImplicit &&
+            CaptureDefault == LCD_ByCopy) {
+          Diag(From.getLocation(), diag::warn_deprecated_this_capture);
+          Diag(CaptureDefaultLoc, diag::note_deprecated_this_capture)
+              << FixItHint::CreateInsertion(
+                     getLocForEndOfToken(CaptureDefaultLoc), ", this");
+        }
+        return LambdaCapture(From.getLocation(), IsImplicit,
+                             From.isCopyCapture() ? LCK_StarThis : LCK_This);
+      } else if (From.isVLATypeCapture()) {
+        return LambdaCapture(From.getLocation(), IsImplicit, LCK_VLAType);
+      } else {
+        assert(From.isVariableCapture() && "unknown kind of capture");
+        ValueDecl *Var = From.getVariable();
+        LambdaCaptureKind Kind = From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef;
+        return LambdaCapture(From.getLocation(), IsImplicit, Kind, Var,
+                             From.getEllipsisLoc());
+      }
+    }();
 
-      // FIXME: Skip this capture if the capture is not used, the initializer
-      // has no side-effects, the type of the capture is trivial, and the
-      // lambda is not externally visible.
+    // Form the initializer for the capture field.
+    ExprResult Init = BuildCaptureInit(From, ImplicitCaptureLoc);
 
-      // Add a FieldDecl for the capture and form its initializer.
-      BuildCaptureField(Class, From);
-      Captures.push_back(Capture);
-      CaptureInits.push_back(Init.get());
+    // FIXME: Skip this capture if the capture is not used, the initializer
+    // has no side-effects, the type of the capture is trivial, and the
+    // lambda is not externally visible.
 
-      if (LangOpts.CUDA)
-        CUDA().CheckLambdaCapture(CallOperator, From);
-    }
+    // Add a FieldDecl for the capture and form its initializer.
+    BuildCaptureField(Class, From);
+    Captures.push_back(Capture);
+    CaptureInits.push_back(Init.get());
 
-    Class->setCaptures(Context, Captures);
-
-    // C++11 [expr.prim.lambda]p6:
-    //   The closure type for a lambda-expression with no lambda-capture
-    //   has a public non-virtual non-explicit const conversion function
-    //   to pointer to function having the same parameter and return
-    //   types as the closure type's function call operator.
-    if (Captures.empty() && CaptureDefault == LCD_None)
-      addFunctionPointerConversions(*this, IntroducerRange, Class,
-                                    CallOperator);
-
-    // Objective-C++:
-    //   The closure type for a lambda-expression has a public non-virtual
-    //   non-explicit const conversion function to a block pointer having the
-    //   same parameter and return types as the closure type's function call
-    //   operator.
-    // FIXME: Fix generic lambda to block conversions.
-    if (getLangOpts().Blocks && getLangOpts().ObjC && !IsGenericLambda)
-      addBlockPointerConversion(*this, IntroducerRange, Class, CallOperator);
-
-    // Finalize the lambda class.
-    SmallVector<Decl*, 4> Fields(Class->fields());
-    ActOnFields(nullptr, Class->getLocation(), Class, Fields, SourceLocation(),
-                SourceLocation(), ParsedAttributesView());
-    CheckCompletedCXXClass(nullptr, Class);
+    if (LangOpts.CUDA)
+      CUDA().CheckLambdaCapture(CallOperator, From);
   }
 
+  Class->setCaptures(Context, Captures);
+
+  // C++11 [expr.prim.lambda]p6:
+  //   The closure type for a lambda-expression with no lambda-capture
+  //   has a public non-virtual non-explicit const conversion function
+  //   to pointer to function having the same parameter and return
+  //   types as the closure type's function call operator.
+  if (Captures.empty() && CaptureDefault == LCD_None)
+    addFunctionPointerConversions(*this, IntroducerRange, Class, CallOperator);
+
+  // Objective-C++:
+  //   The closure type for a lambda-expression has a public non-virtual
+  //   non-explicit const conversion function to a block pointer having the
+  //   same parameter and return types as the closure type's function call
+  //   operator.
+  // FIXME: Fix generic lambda to block conversions.
+  if (getLangOpts().Blocks && getLangOpts().ObjC && !IsGenericLambda)
+    addBlockPointerConversion(*this, IntroducerRange, Class, CallOperator);
+
+  // Finalize the lambda class.
+  SmallVector<Decl *, 4> Fields(Class->fields());
+  ActOnFields(nullptr, Class->getLocation(), Class, Fields, SourceLocation(),
+              SourceLocation(), ParsedAttributesView());
+  CheckCompletedCXXClass(nullptr, Class);
+
   Cleanup.mergeFrom(LambdaCleanup);
 
   LambdaExpr *Lambda =