[clang] [Clang] [Draft] Implement P0588R1 capture rules (PR #105953)

Yupei Liu via cfe-commits cfe-commits at lists.llvm.org
Sat Aug 24 10:54:08 PDT 2024


https://github.com/LYP951018 created https://github.com/llvm/llvm-project/pull/105953


## Code Changes:

- Currently, Clang requires instantiation to determine captures of a packs (refer to 7bf8f6fa8ab123fe97ccd82d9a0ddff85505ee5f), but P0588 allows delayed instantiation of `if constexpr` statements in generic lambdas, which causes packs inside `if constexpr` to not be captured correctly. Therefore, I reverted to the previous approach of instantiating implicit captures in `TransformLambdaExpr` and handled the following two cases:

    ```cpp
    auto v = [](auto... c) {
      sink([&](auto ...b) {
        c;          // case 1
        sink(c...); // case 2
      }...);
    };
    ```

- P0588R1 allows us to determine the set of implicit captures when parsing the lambda expression, so there is no need to call `tryCaptureVariable(/*BuildAndDiagnose=*/true)` during template instantiation. Therefore:
  - The previous concept of "capture-capable" has been removed;
  - Redundant `tryCaptureVariable` calls in potential captures have been removed, but `tryCaptureVariable` in `MarkVarDeclODRUsed` has not been removed yet.

## Behavioral changes

https://godbolt.org/z/e3Wb6hPja

- When there are nested generic lambdas, and whether a variable inside the lambda is odr-used depends on the outer generic lambda (removing the impact of capture-capable):
```cpp
    const int x = 10;
    auto L = [=](auto a) {
        return [=](auto b) {
          DEFINE_SELECTOR(a);
          F_CALL(x, a); 
          return 0;        
        }; 
    };

    auto L0 = L('c');
    static_assert(sizeof(L0) == sizeof(int));
    auto L1 = L(1);
    static_assert(sizeof(L1) == sizeof(int)); // sizeof(L1) is 1 before, now it is 4.
```

- P0588R1 allows us to capture variables referenced inside `typeid`:

```cpp
    auto ltid = [=]{ typeid(x); };
    static_assert(sizeof(ltid) == sizeof(int)); // sizeof(ltid) is 1 before, now it is 4.
```

## Remaining issues

- Is a separate ABI flag needed?
- How to determine if the current variable is within a `typeid` expression in `DoMarkVarDeclReferenced`?

I would appreciate any insights or suggestions on this. Thank you in advance for your help!

partially fixes #61426

>From 952273b27d2204f266b7f5e42cf1bebc755b1d06 Mon Sep 17 00:00:00 2001
From: letrec <liuyupei951018 at hotmail.com>
Date: Sun, 25 Aug 2024 01:28:42 +0800
Subject: [PATCH 1/2] initial draft

---
 clang/include/clang/Sema/Sema.h               |  22 ++
 clang/include/clang/Sema/SemaLambda.h         |  11 -
 clang/lib/Sema/SemaExpr.cpp                   |  11 +-
 clang/lib/Sema/SemaExprCXX.cpp                |  79 ++++---
 clang/lib/Sema/SemaLambda.cpp                 | 209 +-----------------
 clang/lib/Sema/SemaTemplateVariadic.cpp       | 166 +++++++++++++-
 clang/lib/Sema/TreeTransform.h                |  83 +++++--
 .../cxx1y-generic-lambdas-capturing.cpp       |  26 +++
 .../test/SemaTemplate/lambda-capture-pack.cpp |  13 ++
 9 files changed, 351 insertions(+), 269 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1f7e555d1b8717..77f2f7e41d182a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -14111,6 +14111,28 @@ class Sema final : public SemaBase {
   static void collectUnexpandedParameterPacks(
       Expr *E, SmallVectorImpl<UnexpandedParameterPack> &Unexpanded);
 
+  static void collectUnexpandedParameterPacksFromLambdaBody(
+      Stmt *Body, SmallVectorImpl<UnexpandedParameterPack> &Unexpanded);
+
+  static bool containsUnexpandedParameterPacksInLambdaBody(Stmt *Body);
+
+  /// Collect decls expanded inside the lambda body.
+  /// e.g.
+  /// 
+  /// \code
+  ///   auto v = [](auto... c) {
+  ///    sink([&](auto ...b) {
+  ///      c;           // expanded outside the lambda body
+  ///      sink(c...);  // expanded inside the lambda body
+  ///    }...);
+  ///  }(400, 50, 6);
+  /// \endcode
+  ///
+  /// \param Body The lambda body
+  ///
+  static void collectExpandedParameterPacksFromLambdaBody(
+      Stmt *Body, SmallVectorImpl<Decl *> &Expanded);
+
   /// Invoked when parsing a template argument followed by an
   /// ellipsis, which creates a pack expansion.
   ///
diff --git a/clang/include/clang/Sema/SemaLambda.h b/clang/include/clang/Sema/SemaLambda.h
index 3c9d22df70c0df..6ee53959551a1e 100644
--- a/clang/include/clang/Sema/SemaLambda.h
+++ b/clang/include/clang/Sema/SemaLambda.h
@@ -24,17 +24,6 @@ class FunctionScopeInfo;
 }
 class Sema;
 
-/// Examines the FunctionScopeInfo stack to determine the nearest
-/// enclosing lambda (to the current lambda) that is 'capture-capable' for
-/// the variable referenced in the current lambda (i.e. \p VarToCapture).
-/// If successful, returns the index into Sema's FunctionScopeInfo stack
-/// of the capture-capable lambda's LambdaScopeInfo.
-/// See Implementation for more detailed comments.
-
-std::optional<unsigned> getStackIndexOfNearestEnclosingCaptureCapableLambda(
-    ArrayRef<const sema::FunctionScopeInfo *> FunctionScopes,
-    ValueDecl *VarToCapture, Sema &S);
-
 } // clang
 
 #endif
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ea57316ad8014e..e99422e654bb8f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19631,8 +19631,15 @@ static void DoMarkVarDeclReferenced(
   case OdrUseContext::Dependent:
     // If this is a dependent context, we don't need to mark variables as
     // odr-used, but we may still need to track them for lambda capture.
-    // FIXME: Do we also need to do this inside dependent typeid expressions
-    // (which are modeled as unevaluated at this point)?
+    // 
+    // If an expression potentially references a local entity within a
+    // declarative region in which it is odr-usable, and the expression would be
+    // potentially evaluated if the effect of any enclosing typeid expressions
+    // ([expr.typeid]) were ignored, the entity is said to be implicitly
+    // captured by each intervening lambda-expression with an associated
+    // capture-default that does not explicitly capture it.
+    // TODO: How to determine if the current variable is within a typeid expression?
+
     DoMarkPotentialCapture(SemaRef, Loc, Var, E);
     break;
   }
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 746c67ff1e979f..fd1b6d9f2d6542 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8659,6 +8659,11 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
   // lambda (within a generic outer lambda), must be captured by an
   // outer lambda that is enclosed within a non-dependent context.
   CurrentLSI->visitPotentialCaptures([&](ValueDecl *Var, Expr *VarExpr) {
+    VarDecl *UnderlyingVar = Var->getPotentiallyDecomposedVarDecl();
+    if (!UnderlyingVar)
+      return;
+
+
     // If the variable is clearly identified as non-odr-used and the full
     // expression is not instantiation dependent, only then do we not
     // need to check enclosing lambda's for speculative captures.
@@ -8674,52 +8679,58 @@ static void CheckIfAnyEnclosingLambdasMustCaptureAnyPotentialCaptures(
         !IsFullExprInstantiationDependent)
       return;
 
-    VarDecl *UnderlyingVar = Var->getPotentiallyDecomposedVarDecl();
-    if (!UnderlyingVar)
-      return;
-
-    // If we have a capture-capable lambda for the variable, go ahead and
-    // capture the variable in that lambda (and all its enclosing lambdas).
-    if (const std::optional<unsigned> Index =
-            getStackIndexOfNearestEnclosingCaptureCapableLambda(
-                S.FunctionScopes, Var, S))
-      S.MarkCaptureUsedInEnclosingContext(Var, VarExpr->getExprLoc(), *Index);
-    const bool IsVarNeverAConstantExpression =
-        VariableCanNeverBeAConstantExpression(UnderlyingVar, S.Context);
-    if (!IsFullExprInstantiationDependent || IsVarNeverAConstantExpression) {
-      // This full expression is not instantiation dependent or the variable
-      // can not be used in a constant expression - which means
-      // this variable must be odr-used here, so diagnose a
-      // capture violation early, if the variable is un-captureable.
-      // This is purely for diagnosing errors early.  Otherwise, this
-      // error would get diagnosed when the lambda becomes capture ready.
+    // we already complete the captures of lambdas when parsing.
+    // so we dont capture any new vars in template instantiation
+    if (!S.inTemplateInstantiation()) {
       QualType CaptureType, DeclRefType;
       SourceLocation ExprLoc = VarExpr->getExprLoc();
-      if (S.tryCaptureVariable(Var, ExprLoc, S.TryCapture_Implicit,
-                          /*EllipsisLoc*/ SourceLocation(),
-                          /*BuildAndDiagnose*/false, CaptureType,
-                          DeclRefType, nullptr)) {
-        // We will never be able to capture this variable, and we need
-        // to be able to in any and all instantiations, so diagnose it.
+      if (!S.tryCaptureVariable(Var, ExprLoc, S.TryCapture_Implicit,
+                                /*EllipsisLoc*/ SourceLocation(),
+                                /*BuildAndDiagnose*/ false, CaptureType,
+                                DeclRefType, nullptr)) {
         S.tryCaptureVariable(Var, ExprLoc, S.TryCapture_Implicit,
-                          /*EllipsisLoc*/ SourceLocation(),
-                          /*BuildAndDiagnose*/true, CaptureType,
-                          DeclRefType, nullptr);
+                             /*EllipsisLoc*/ SourceLocation(),
+                             /*BuildAndDiagnose*/ true, CaptureType,
+                             DeclRefType, nullptr);
       }
     }
+
+
+     const bool IsVarNeverAConstantExpression =
+          VariableCanNeverBeAConstantExpression(UnderlyingVar, S.Context);
+      if (!IsFullExprInstantiationDependent || IsVarNeverAConstantExpression) {
+        // This full expression is not instantiation dependent or the variable
+        // can not be used in a constant expression - which means
+        // this variable must be odr-used here, so diagnose a
+        // capture violation early, if the variable is un-captureable.
+        // This is purely for diagnosing errors early.  Otherwise, this
+        // error would get diagnosed when the lambda becomes capture ready.
+        QualType CaptureType, DeclRefType;
+        SourceLocation ExprLoc = VarExpr->getExprLoc();
+        if (S.tryCaptureVariable(Var, ExprLoc, S.TryCapture_Implicit,
+                                 /*EllipsisLoc*/ SourceLocation(),
+                                 /*BuildAndDiagnose*/ false, CaptureType,
+                                 DeclRefType, nullptr)) {
+          // We will never be able to capture this variable, and we need
+          // to be able to in any and all instantiations, so diagnose it.
+          S.tryCaptureVariable(Var, ExprLoc, S.TryCapture_Implicit,
+                               /*EllipsisLoc*/ SourceLocation(),
+                               /*BuildAndDiagnose*/ true, CaptureType,
+                               DeclRefType, nullptr);
+        }
+      }
   });
 
   // Check if 'this' needs to be captured.
-  if (CurrentLSI->hasPotentialThisCapture()) {
+  if (CurrentLSI->hasPotentialThisCapture() && !S.inTemplateInstantiation()) {
     // If we have a capture-capable lambda for 'this', go ahead and capture
     // 'this' in that lambda (and all its enclosing lambdas).
-    if (const std::optional<unsigned> Index =
-            getStackIndexOfNearestEnclosingCaptureCapableLambda(
-                S.FunctionScopes, /*0 is 'this'*/ nullptr, S)) {
-      const unsigned FunctionScopeIndexOfCapturableLambda = *Index;
+    if (!S.CheckCXXThisCapture(CurrentLSI->PotentialThisCaptureLocation,
+                               /*Explicit*/ false, /*BuildAndDiagnose*/ false,
+                               nullptr)) {
       S.CheckCXXThisCapture(CurrentLSI->PotentialThisCaptureLocation,
                             /*Explicit*/ false, /*BuildAndDiagnose*/ true,
-                            &FunctionScopeIndexOfCapturableLambda);
+                            nullptr);
     }
   }
 
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index a8461a7f6cf712..7901a3b47f46a2 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -30,207 +30,6 @@
 using namespace clang;
 using namespace sema;
 
-/// Examines the FunctionScopeInfo stack to determine the nearest
-/// enclosing lambda (to the current lambda) that is 'capture-ready' for
-/// the variable referenced in the current lambda (i.e. \p VarToCapture).
-/// If successful, returns the index into Sema's FunctionScopeInfo stack
-/// of the capture-ready lambda's LambdaScopeInfo.
-///
-/// Climbs down the stack of lambdas (deepest nested lambda - i.e. current
-/// lambda - is on top) to determine the index of the nearest enclosing/outer
-/// lambda that is ready to capture the \p VarToCapture being referenced in
-/// the current lambda.
-/// As we climb down the stack, we want the index of the first such lambda -
-/// that is the lambda with the highest index that is 'capture-ready'.
-///
-/// A lambda 'L' is capture-ready for 'V' (var or this) if:
-///  - its enclosing context is non-dependent
-///  - and if the chain of lambdas between L and the lambda in which
-///    V is potentially used (i.e. the lambda at the top of the scope info
-///    stack), can all capture or have already captured V.
-/// If \p VarToCapture is 'null' then we are trying to capture 'this'.
-///
-/// Note that a lambda that is deemed 'capture-ready' still needs to be checked
-/// for whether it is 'capture-capable' (see
-/// getStackIndexOfNearestEnclosingCaptureCapableLambda), before it can truly
-/// capture.
-///
-/// \param FunctionScopes - Sema's stack of nested FunctionScopeInfo's (which a
-///  LambdaScopeInfo inherits from).  The current/deepest/innermost lambda
-///  is at the top of the stack and has the highest index.
-/// \param VarToCapture - the variable to capture.  If NULL, capture 'this'.
-///
-/// \returns An std::optional<unsigned> Index that if evaluates to 'true'
-/// contains the index (into Sema's FunctionScopeInfo stack) of the innermost
-/// lambda which is capture-ready.  If the return value evaluates to 'false'
-/// then no lambda is capture-ready for \p VarToCapture.
-
-static inline std::optional<unsigned>
-getStackIndexOfNearestEnclosingCaptureReadyLambda(
-    ArrayRef<const clang::sema::FunctionScopeInfo *> FunctionScopes,
-    ValueDecl *VarToCapture) {
-  // Label failure to capture.
-  const std::optional<unsigned> NoLambdaIsCaptureReady;
-
-  // Ignore all inner captured regions.
-  unsigned CurScopeIndex = FunctionScopes.size() - 1;
-  while (CurScopeIndex > 0 && isa<clang::sema::CapturedRegionScopeInfo>(
-                                  FunctionScopes[CurScopeIndex]))
-    --CurScopeIndex;
-  assert(
-      isa<clang::sema::LambdaScopeInfo>(FunctionScopes[CurScopeIndex]) &&
-      "The function on the top of sema's function-info stack must be a lambda");
-
-  // If VarToCapture is null, we are attempting to capture 'this'.
-  const bool IsCapturingThis = !VarToCapture;
-  const bool IsCapturingVariable = !IsCapturingThis;
-
-  // Start with the current lambda at the top of the stack (highest index).
-  DeclContext *EnclosingDC =
-      cast<sema::LambdaScopeInfo>(FunctionScopes[CurScopeIndex])->CallOperator;
-
-  do {
-    const clang::sema::LambdaScopeInfo *LSI =
-        cast<sema::LambdaScopeInfo>(FunctionScopes[CurScopeIndex]);
-    // IF we have climbed down to an intervening enclosing lambda that contains
-    // the variable declaration - it obviously can/must not capture the
-    // variable.
-    // Since its enclosing DC is dependent, all the lambdas between it and the
-    // innermost nested lambda are dependent (otherwise we wouldn't have
-    // arrived here) - so we don't yet have a lambda that can capture the
-    // variable.
-    if (IsCapturingVariable &&
-        VarToCapture->getDeclContext()->Equals(EnclosingDC))
-      return NoLambdaIsCaptureReady;
-
-    // For an enclosing lambda to be capture ready for an entity, all
-    // intervening lambda's have to be able to capture that entity. If even
-    // one of the intervening lambda's is not capable of capturing the entity
-    // then no enclosing lambda can ever capture that entity.
-    // For e.g.
-    // const int x = 10;
-    // [=](auto a) {    #1
-    //   [](auto b) {   #2 <-- an intervening lambda that can never capture 'x'
-    //    [=](auto c) { #3
-    //       f(x, c);  <-- can not lead to x's speculative capture by #1 or #2
-    //    }; }; };
-    // If they do not have a default implicit capture, check to see
-    // if the entity has already been explicitly captured.
-    // If even a single dependent enclosing lambda lacks the capability
-    // to ever capture this variable, there is no further enclosing
-    // non-dependent lambda that can capture this variable.
-    if (LSI->ImpCaptureStyle == sema::LambdaScopeInfo::ImpCap_None) {
-      if (IsCapturingVariable && !LSI->isCaptured(VarToCapture))
-        return NoLambdaIsCaptureReady;
-      if (IsCapturingThis && !LSI->isCXXThisCaptured())
-        return NoLambdaIsCaptureReady;
-    }
-    EnclosingDC = getLambdaAwareParentOfDeclContext(EnclosingDC);
-
-    assert(CurScopeIndex);
-    --CurScopeIndex;
-  } while (!EnclosingDC->isTranslationUnit() &&
-           EnclosingDC->isDependentContext() &&
-           isLambdaCallOperator(EnclosingDC));
-
-  assert(CurScopeIndex < (FunctionScopes.size() - 1));
-  // If the enclosingDC is not dependent, then the immediately nested lambda
-  // (one index above) is capture-ready.
-  if (!EnclosingDC->isDependentContext())
-    return CurScopeIndex + 1;
-  return NoLambdaIsCaptureReady;
-}
-
-/// Examines the FunctionScopeInfo stack to determine the nearest
-/// enclosing lambda (to the current lambda) that is 'capture-capable' for
-/// the variable referenced in the current lambda (i.e. \p VarToCapture).
-/// If successful, returns the index into Sema's FunctionScopeInfo stack
-/// of the capture-capable lambda's LambdaScopeInfo.
-///
-/// Given the current stack of lambdas being processed by Sema and
-/// the variable of interest, to identify the nearest enclosing lambda (to the
-/// current lambda at the top of the stack) that can truly capture
-/// a variable, it has to have the following two properties:
-///  a) 'capture-ready' - be the innermost lambda that is 'capture-ready':
-///     - climb down the stack (i.e. starting from the innermost and examining
-///       each outer lambda step by step) checking if each enclosing
-///       lambda can either implicitly or explicitly capture the variable.
-///       Record the first such lambda that is enclosed in a non-dependent
-///       context. If no such lambda currently exists return failure.
-///  b) 'capture-capable' - make sure the 'capture-ready' lambda can truly
-///  capture the variable by checking all its enclosing lambdas:
-///     - check if all outer lambdas enclosing the 'capture-ready' lambda
-///       identified above in 'a' can also capture the variable (this is done
-///       via tryCaptureVariable for variables and CheckCXXThisCapture for
-///       'this' by passing in the index of the Lambda identified in step 'a')
-///
-/// \param FunctionScopes - Sema's stack of nested FunctionScopeInfo's (which a
-/// LambdaScopeInfo inherits from).  The current/deepest/innermost lambda
-/// is at the top of the stack.
-///
-/// \param VarToCapture - the variable to capture.  If NULL, capture 'this'.
-///
-///
-/// \returns An std::optional<unsigned> Index that if evaluates to 'true'
-/// contains the index (into Sema's FunctionScopeInfo stack) of the innermost
-/// lambda which is capture-capable.  If the return value evaluates to 'false'
-/// then no lambda is capture-capable for \p VarToCapture.
-
-std::optional<unsigned>
-clang::getStackIndexOfNearestEnclosingCaptureCapableLambda(
-    ArrayRef<const sema::FunctionScopeInfo *> FunctionScopes,
-    ValueDecl *VarToCapture, Sema &S) {
-
-  const std::optional<unsigned> NoLambdaIsCaptureCapable;
-
-  const std::optional<unsigned> OptionalStackIndex =
-      getStackIndexOfNearestEnclosingCaptureReadyLambda(FunctionScopes,
-                                                        VarToCapture);
-  if (!OptionalStackIndex)
-    return NoLambdaIsCaptureCapable;
-
-  const unsigned IndexOfCaptureReadyLambda = *OptionalStackIndex;
-  assert(((IndexOfCaptureReadyLambda != (FunctionScopes.size() - 1)) ||
-          S.getCurGenericLambda()) &&
-         "The capture ready lambda for a potential capture can only be the "
-         "current lambda if it is a generic lambda");
-
-  const sema::LambdaScopeInfo *const CaptureReadyLambdaLSI =
-      cast<sema::LambdaScopeInfo>(FunctionScopes[IndexOfCaptureReadyLambda]);
-
-  // If VarToCapture is null, we are attempting to capture 'this'
-  const bool IsCapturingThis = !VarToCapture;
-  const bool IsCapturingVariable = !IsCapturingThis;
-
-  if (IsCapturingVariable) {
-    // Check if the capture-ready lambda can truly capture the variable, by
-    // checking whether all enclosing lambdas of the capture-ready lambda allow
-    // the capture - i.e. make sure it is capture-capable.
-    QualType CaptureType, DeclRefType;
-    const bool CanCaptureVariable =
-        !S.tryCaptureVariable(VarToCapture,
-                              /*ExprVarIsUsedInLoc*/ SourceLocation(),
-                              clang::Sema::TryCapture_Implicit,
-                              /*EllipsisLoc*/ SourceLocation(),
-                              /*BuildAndDiagnose*/ false, CaptureType,
-                              DeclRefType, &IndexOfCaptureReadyLambda);
-    if (!CanCaptureVariable)
-      return NoLambdaIsCaptureCapable;
-  } else {
-    // Check if the capture-ready lambda can truly capture 'this' by checking
-    // whether all enclosing lambdas of the capture-ready lambda can capture
-    // 'this'.
-    const bool CanCaptureThis =
-        !S.CheckCXXThisCapture(
-             CaptureReadyLambdaLSI->PotentialThisCaptureLocation,
-             /*Explicit*/ false, /*BuildAndDiagnose*/ false,
-             &IndexOfCaptureReadyLambda);
-    if (!CanCaptureThis)
-      return NoLambdaIsCaptureCapable;
-  }
-  return IndexOfCaptureReadyLambda;
-}
-
 static inline TemplateParameterList *
 getGenericLambdaTemplateParameterList(LambdaScopeInfo *LSI, Sema &SemaRef) {
   if (!LSI->GLTemplateParameterList && !LSI->TemplateParams.empty()) {
@@ -2126,11 +1925,15 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
 
       // Use source ranges of explicit captures for fixits where available.
       SourceRange CaptureRange = LSI->ExplicitCaptureRanges[I];
-
+      bool PotentiallyCaptured = false;
+      LSI->visitPotentialCaptures([&](ValueDecl *Var, Expr *VarExpr) {
+        if (Var == From.getVariable())
+          PotentiallyCaptured = true;
+      });
       // Warn about unused explicit captures.
       bool IsCaptureUsed = true;
       if (!CurContext->isDependentContext() && !IsImplicit &&
-          !From.isODRUsed()) {
+          !From.isODRUsed() && !PotentiallyCaptured) {
         // Initialized captures that are non-ODR used may not be eliminated.
         // FIXME: Where did the IsGenericLambda here come from?
         bool NonODRUsedInitCapture =
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index bcd31c98871e22..7817f87b885af1 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -69,6 +69,8 @@ namespace {
     // We need this so we can find e.g. attributes on lambdas.
     bool shouldVisitImplicitCode() const { return true; }
 
+    void SetInLambda(bool b) { InLambda = true; }
+
     //------------------------------------------------------------------------
     // Recording occurrences of (unexpanded) parameter packs.
     //------------------------------------------------------------------------
@@ -281,7 +283,150 @@ namespace {
       return inherited::TraverseLambdaCapture(Lambda, C, Init);
     }
   };
-}
+
+
+  class CollectExpandedParameterPacksVisitor
+      : public RecursiveASTVisitor<CollectExpandedParameterPacksVisitor> {
+    typedef RecursiveASTVisitor<CollectExpandedParameterPacksVisitor> inherited;
+
+    SmallVectorImpl<Decl*> &Expanded;
+    bool UnderExpanded = false;
+
+    struct UnderExpandedRAII {
+      bool &UnderExpanded;
+      bool Old;
+      UnderExpandedRAII(bool &NewUnderExpanded)
+          : UnderExpanded(NewUnderExpanded), Old{NewUnderExpanded} {
+        UnderExpanded = true;
+      }
+      ~UnderExpandedRAII() { UnderExpanded = Old; }
+    };
+
+
+     bool InLambda = false;
+    unsigned DepthLimit = (unsigned)-1;
+
+    void addExpanded(Decl *VD, SourceLocation Loc = SourceLocation()) {
+      Expanded.push_back(VD);
+    }
+
+  public:
+    explicit CollectExpandedParameterPacksVisitor(
+        SmallVectorImpl<Decl *> &Expanded)
+        : Expanded(Expanded) {}
+
+    bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+    // We need this so we can find e.g. attributes on lambdas.
+    bool shouldVisitImplicitCode() const { return false; }
+
+    void SetInLambda(bool b) { InLambda = true; }
+
+    bool VisitDeclRefExpr(DeclRefExpr *E) {
+      if (E->getDecl()->isParameterPack() && UnderExpanded)
+        addExpanded(E->getDecl(), E->getLocation());
+
+      return true;
+    }
+
+    /// Suppress traversal into Objective-C container literal
+    /// elements that are pack expansions.
+    bool TraverseObjCDictionaryLiteral(ObjCDictionaryLiteral *E) {
+      for (unsigned I = 0, N = E->getNumElements(); I != N; ++I) {
+        ObjCDictionaryElement Element = E->getKeyValueElement(I);
+        if (Element.isPackExpansion()) {
+          UnderExpandedRAII UnderExpandedRAII{UnderExpanded};
+          TraverseStmt(Element.Key);
+          TraverseStmt(Element.Value);
+        }
+      }
+      return true;
+    }
+    /// Suppress traversal of parameter packs.
+    bool TraverseDecl(Decl *D) {
+      // A function parameter pack is a pack expansion, so cannot contain
+      // an unexpanded parameter pack. Likewise for a template parameter
+      // pack that contains any references to other packs.
+      if (D && D->isParameterPack()) {
+        UnderExpandedRAII UnderExpandedRAII{UnderExpanded};
+        return inherited::TraverseDecl(D);
+      }
+      return true;
+    }
+
+    bool TraversePackExpansionExpr(PackExpansionExpr *E) { 
+      UnderExpandedRAII UnderExpandedRAII{UnderExpanded};
+      return inherited::TraversePackExpansionExpr(E);
+    }
+    bool TraverseCXXFoldExpr(CXXFoldExpr *E) { 
+      UnderExpandedRAII UnderExpandedRAII{UnderExpanded};
+      return inherited::TraverseCXXFoldExpr(E);
+    }
+    // TODO?
+    bool TraversePackIndexingExpr(PackIndexingExpr *E) {
+      return inherited::TraverseStmt(E->getIndexExpr());
+    }
+
+    // TODO: MemberExpr & FunctionParmPackExpr
+
+    /// Suppress traversal of using-declaration pack expansion.
+    bool TraverseUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D) {
+      if (D->isPackExpansion())
+        return true;
+
+      return inherited::TraverseUnresolvedUsingValueDecl(D);
+    }
+
+    /// Suppress traversal of using-declaration pack expansion.
+    bool TraverseUnresolvedUsingTypenameDecl(UnresolvedUsingTypenameDecl *D) {
+      if (D->isPackExpansion())
+        return true;
+
+      return inherited::TraverseUnresolvedUsingTypenameDecl(D);
+    }
+
+    /// Suppress traversal of template argument pack expansions.
+    bool TraverseTemplateArgument(const TemplateArgument &Arg) {
+      if (Arg.isPackExpansion())
+        return true;
+
+      return inherited::TraverseTemplateArgument(Arg);
+    }
+
+    /// Suppress traversal of template argument pack expansions.
+    bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &ArgLoc) {
+      if (ArgLoc.getArgument().isPackExpansion())
+        return true;
+
+      return inherited::TraverseTemplateArgumentLoc(ArgLoc);
+    }
+
+    /// Suppress traversal of base specifier pack expansions.
+    bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &Base) {
+      if (Base.isPackExpansion())
+        return true;
+
+      return inherited::TraverseCXXBaseSpecifier(Base);
+    }
+
+    /// Suppress traversal of mem-initializer pack expansions.
+    bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
+      if (Init->isPackExpansion())
+        return true;
+
+      return inherited::TraverseConstructorInitializer(Init);
+    }
+
+    /// Note whether we're traversing a lambda containing an unexpanded
+    /// parameter pack. In this case, the unexpanded pack can occur anywhere,
+    /// including all the places where we normally wouldn't look. Within a
+    /// lambda, we don't propagate the 'contains unexpanded parameter pack' bit
+    /// outside an expression.
+    bool TraverseLambdaExpr(LambdaExpr *Lambda) {
+      return true;
+    }
+  };
+  }
 
 /// Determine whether it's possible for an unexpanded parameter pack to
 /// be valid in this location. This only happens when we're in a declaration
@@ -571,6 +716,25 @@ void Sema::collectUnexpandedParameterPacks(
   CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseStmt(E);
 }
 
+void Sema::collectUnexpandedParameterPacksFromLambdaBody(
+    Stmt *Body, SmallVectorImpl<UnexpandedParameterPack> &Unexpanded) {
+  CollectUnexpandedParameterPacksVisitor visitor(Unexpanded);
+  visitor.SetInLambda(true);
+  visitor.TraverseStmt(Body);
+}
+
+void Sema::collectExpandedParameterPacksFromLambdaBody(
+      Stmt *Body, SmallVectorImpl<Decl *> &Expanded) {
+  CollectExpandedParameterPacksVisitor visitor(Expanded);
+  visitor.TraverseStmt(Body);
+}
+
+bool Sema::containsUnexpandedParameterPacksInLambdaBody(Stmt *Body) {
+  SmallVector<UnexpandedParameterPack, 2> Unexpanded;
+  collectUnexpandedParameterPacksFromLambdaBody(Body, Unexpanded);
+  return !Unexpanded.empty();
+}
+
 ParsedTemplateArgument
 Sema::ActOnPackExpansion(const ParsedTemplateArgument &Arg,
                          SourceLocation EllipsisLoc) {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 62287c2d26375c..7d8861444fbfd4 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14568,15 +14568,30 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
                                  /*NewThisContext*/false);
 
   bool Invalid = false;
+  bool ContainsUnexpandedInBody =
+      E->containsUnexpandedParameterPack() &&
+      Sema::containsUnexpandedParameterPacksInLambdaBody(E->getBody());
+
+  SmallVector<Decl *, 1> PacksExpandedInBody;
+  if (ContainsUnexpandedInBody) {
+    SemaRef.collectExpandedParameterPacksFromLambdaBody(E->getBody(),
+                                                        PacksExpandedInBody);
+  }
+
 
   // Transform captures.
+  bool FinishedExplicitCaptures = false;
   for (LambdaExpr::capture_iterator C = E->capture_begin(),
                                  CEnd = E->capture_end();
        C != CEnd; ++C) {
     // When we hit the first implicit capture, tell Sema that we've finished
     // the list of explicit captures.
-    if (C->isImplicit())
-      break;
+    if (C->isImplicit()) {
+      if (!FinishedExplicitCaptures) {
+        getSema().finishLambdaExplicitCaptures(LSI);
+      }
+      FinishedExplicitCaptures = true;
+    }
 
     // Capturing 'this' is trivial.
     if (C->capturesThis()) {
@@ -14649,12 +14664,15 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
                            ? Sema::TryCapture_ExplicitByVal
                            : Sema::TryCapture_ExplicitByRef;
     SourceLocation EllipsisLoc;
-    if (C->isPackExpansion()) {
+    const bool IsImplicitCapturePack =
+        C->isImplicit() &&
+        C->getCapturedVar()->getType()->getAs<PackExpansionType>();
+    if (C->isPackExpansion() || IsImplicitCapturePack) {
       UnexpandedParameterPack Unexpanded(C->getCapturedVar(), C->getLocation());
       bool ShouldExpand = false;
       bool RetainExpansion = false;
       std::optional<unsigned> NumExpansions;
-      if (getDerived().TryExpandParameterPacks(C->getEllipsisLoc(),
+      if (getDerived().TryExpandParameterPacks(IsImplicitCapturePack ? C->getLocation() : C->getEllipsisLoc(),
                                                C->getLocation(),
                                                Unexpanded,
                                                ShouldExpand, RetainExpansion,
@@ -14663,31 +14681,58 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         continue;
       }
 
+
+
       if (ShouldExpand) {
         // The transform has determined that we should perform an expansion;
         // transform and capture each of the arguments.
         // expansion of the pattern. Do so.
         auto *Pack = cast<VarDecl>(C->getCapturedVar());
-        for (unsigned I = 0; I != *NumExpansions; ++I) {
-          Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
-          VarDecl *CapturedVar
-            = cast_or_null<VarDecl>(getDerived().TransformDecl(C->getLocation(),
-                                                               Pack));
-          if (!CapturedVar) {
-            Invalid = true;
-            continue;
-          }
+        // Transform the implicitly and explicitly captured packs in a lambda.
+        // For the implicit capture case, there are two forms:
+        // 1. [&] {
+        //      sink(x...)
+        //    }
+        //    We need to capture all x...
+        // 2. [](auto... c) {
+        //      sink([&]() {
+        //        c;
+        //      }...);
+        //    }
+        //    We need to capture the correct c[index] of the pack.
+        if (!IsImplicitCapturePack || !ContainsUnexpandedInBody ||
+            llvm::is_contained(PacksExpandedInBody, Pack)) {
+          for (unsigned I = 0; I != *NumExpansions; ++I) {
+            Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
+            VarDecl *CapturedVar = cast_or_null<VarDecl>(
+                getDerived().TransformDecl(C->getLocation(), Pack));
+            if (!CapturedVar) {
+              Invalid = true;
+              continue;
+            }
 
-          // Capture the transformed variable.
-          getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind);
+            // Capture the transformed variable.
+            getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind);
+          }
+        } else {
+          if (*NumExpansions > 0) {
+            assert(getSema().ArgumentPackSubstitutionIndex != -1);
+            VarDecl *CapturedVar = cast_or_null<VarDecl>(
+                getDerived().TransformDecl(C->getLocation(), Pack));
+            if (!CapturedVar) {
+              Invalid = true;
+            } else {
+              getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind);
+            }
+          }
         }
 
-        // FIXME: Retain a pack expansion if RetainExpansion is true.
 
+        // FIXME: Retain a pack expansion if RetainExpansion is true.
         continue;
       }
 
-      EllipsisLoc = C->getEllipsisLoc();
+      EllipsisLoc = IsImplicitCapturePack ? C->getLocation() : C->getEllipsisLoc();
     }
 
     // Transform the captured variable.
@@ -14707,8 +14752,10 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
                                  EllipsisLoc);
   }
-  getSema().finishLambdaExplicitCaptures(LSI);
 
+  if (!FinishedExplicitCaptures)
+    getSema().finishLambdaExplicitCaptures(LSI);
+  
   // Transform the template parameters, and add them to the current
   // instantiation scope. The null case is handled correctly.
   auto TPL = getDerived().TransformTemplateParameterList(
diff --git a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
index b234c541a20398..eef07d79770cf6 100644
--- a/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
+++ b/clang/test/SemaCXX/cxx1y-generic-lambdas-capturing.cpp
@@ -1543,3 +1543,29 @@ namespace capture_pack {
   static_assert(v == 123);
 #endif
 }
+
+namespace std {
+  class type_info;
+}
+
+namespace p0588r1 {
+void doit() {
+  // behavior changes since P0588R1
+  const int x = 10;
+  auto L = [=](auto a) {
+    return [=](auto b) {
+      DEFINE_SELECTOR(a);
+      F_CALL(x, a);
+      return 0;
+    };
+  };
+
+  auto L0 = L('c');
+  ASSERT_CLOSURE_SIZE_EXACT(L0, sizeof(int));
+  auto L1 = L(1);
+  ASSERT_CLOSURE_SIZE_EXACT(L1, sizeof(int));
+
+  auto ltid = [=] { typeid(x); };
+  ASSERT_CLOSURE_SIZE_EXACT(ltid, sizeof(int));
+}
+}
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 35b2ffcefea355..400437b4cfecea 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -23,3 +23,16 @@ namespace PR41576 {
   }
   static_assert(f(3, 4) == 6); // expected-note {{instantiation}}
 }
+
+namespace multi_unpack {
+template <typename... Args> void sink(Args...) {}
+void f() {
+  [](auto... c) {
+    check_sizes<int[3], int[3], int[3]>([=](auto... b) {
+      c;
+      sink(c...);
+      return c;
+    }...);
+  }(400, 60, 3);
+}
+} // namespace multi_unpack

>From afdff4e15c518984206da5b7cdbe9b14bb1b3bea Mon Sep 17 00:00:00 2001
From: letrec <liuyupei951018 at hotmail.com>
Date: Sun, 25 Aug 2024 01:53:11 +0800
Subject: [PATCH 2/2] Remove unnecessary visits

---
 clang/lib/Sema/SemaTemplateVariadic.cpp | 39 ++-----------------------
 1 file changed, 2 insertions(+), 37 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 7817f87b885af1..c6d34116183160 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -369,38 +369,7 @@ namespace {
 
     // TODO: MemberExpr & FunctionParmPackExpr
 
-    /// Suppress traversal of using-declaration pack expansion.
-    bool TraverseUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D) {
-      if (D->isPackExpansion())
-        return true;
-
-      return inherited::TraverseUnresolvedUsingValueDecl(D);
-    }
-
-    /// Suppress traversal of using-declaration pack expansion.
-    bool TraverseUnresolvedUsingTypenameDecl(UnresolvedUsingTypenameDecl *D) {
-      if (D->isPackExpansion())
-        return true;
-
-      return inherited::TraverseUnresolvedUsingTypenameDecl(D);
-    }
-
-    /// Suppress traversal of template argument pack expansions.
-    bool TraverseTemplateArgument(const TemplateArgument &Arg) {
-      if (Arg.isPackExpansion())
-        return true;
-
-      return inherited::TraverseTemplateArgument(Arg);
-    }
-
-    /// Suppress traversal of template argument pack expansions.
-    bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &ArgLoc) {
-      if (ArgLoc.getArgument().isPackExpansion())
-        return true;
-
-      return inherited::TraverseTemplateArgumentLoc(ArgLoc);
-    }
-
+    // TODO:
     /// Suppress traversal of base specifier pack expansions.
     bool TraverseCXXBaseSpecifier(const CXXBaseSpecifier &Base) {
       if (Base.isPackExpansion())
@@ -417,11 +386,7 @@ namespace {
       return inherited::TraverseConstructorInitializer(Init);
     }
 
-    /// Note whether we're traversing a lambda containing an unexpanded
-    /// parameter pack. In this case, the unexpanded pack can occur anywhere,
-    /// including all the places where we normally wouldn't look. Within a
-    /// lambda, we don't propagate the 'contains unexpanded parameter pack' bit
-    /// outside an expression.
+    /// we don't need to traverse into lambdas
     bool TraverseLambdaExpr(LambdaExpr *Lambda) {
       return true;
     }



More information about the cfe-commits mailing list