[clang] [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (PR #86265)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Thu Jul 18 00:27:12 PDT 2024


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/86265

>From 6e7b38b3e3f781e11db2fa5d552fdfb6123609df Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 22 Mar 2024 17:34:08 +0800
Subject: [PATCH 01/10] [Sema] Preserve ContainsUnexpandedParameterPack in
 TransformLambdaExpr

---
 clang/docs/ReleaseNotes.rst                   |  2 +
 clang/include/clang/Sema/Sema.h               |  8 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp       | 16 +++++-
 clang/lib/Sema/TreeTransform.h                | 18 ++++++
 .../test/SemaTemplate/lambda-capture-pack.cpp | 57 +++++++++++++++++++
 5 files changed, 98 insertions(+), 3 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 690fc7ed271a3..2d11a4b8c092a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -344,6 +344,8 @@ Bug Fixes to C++ Support
   when one of the function had more specialized templates.
   Fixes (`#82509 <https://github.com/llvm/llvm-project/issues/82509>`_)
   and (`#74494 <https://github.com/llvm/llvm-project/issues/74494>`_)
+- Fixed a crash where template parameter packs were not expanded correctly in a lambda being
+  used as a pattern of a folded expression. (#GH56852), (#GH85667)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index cfc1c3b349478..0d1a2e54840e5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11201,6 +11201,14 @@ class Sema final {
   void collectUnexpandedParameterPacks(
       QualType T, SmallVectorImpl<UnexpandedParameterPack> &Unexpanded);
 
+  /// Collect the set of unexpanded parameter packs from a lambda call operator.
+  ///
+  /// \param LambdaCall The lambda call operator that will be traversed to find
+  /// unexpanded parameter packs.
+  void collectUnexpandedParameterPacksFromLambda(
+      CXXMethodDecl *LambdaCall,
+      SmallVectorImpl<UnexpandedParameterPack> &Unexpanded);
+
   /// Collect the set of unexpanded parameter packs within the given
   /// type.
   ///
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 903fbfd18e779..638ceac4148ae 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -8,14 +8,15 @@
 //  This file implements semantic analysis for C++0x variadic templates.
 //===----------------------------------------------------------------------===/
 
-#include "clang/Sema/Sema.h"
 #include "TypeLocBuilder.h"
+#include "clang/AST/ASTLambda.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/ScopeInfo.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include <optional>
@@ -61,8 +62,9 @@ namespace {
 
   public:
     explicit CollectUnexpandedParameterPacksVisitor(
-        SmallVectorImpl<UnexpandedParameterPack> &Unexpanded)
-        : Unexpanded(Unexpanded) {}
+        SmallVectorImpl<UnexpandedParameterPack> &Unexpanded,
+        bool InLambda = false)
+        : Unexpanded(Unexpanded), InLambda(InLambda) {}
 
     bool shouldWalkTypesOfTypeLocs() const { return false; }
 
@@ -544,6 +546,14 @@ void Sema::collectUnexpandedParameterPacks(QualType T,
   CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseType(T);
 }
 
+void Sema::collectUnexpandedParameterPacksFromLambda(
+    CXXMethodDecl *LambdaCall,
+    SmallVectorImpl<UnexpandedParameterPack> &Unexpanded) {
+  assert(isLambdaCallOperator(LambdaCall) && "Expected a lambda call operator");
+  CollectUnexpandedParameterPacksVisitor(Unexpanded, /*InLambda=*/true)
+      .TraverseDecl(LambdaCall);
+}
+
 void Sema::collectUnexpandedParameterPacks(TypeLoc TL,
                    SmallVectorImpl<UnexpandedParameterPack> &Unexpanded) {
   CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseTypeLoc(TL);
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 2d22692f3ab75..f5a859c57034a 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13748,6 +13748,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         }
         NewVDs.push_back(NewVD);
         getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
+        LSI->ContainsUnexpandedParameterPack |=
+            Init.get()->containsUnexpandedParameterPack();
       }
 
       if (Invalid)
@@ -13936,6 +13938,22 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
                                     /*IsInstantiation*/ true);
   SavedContext.pop();
 
+  // The lambda may contain a pack that would be expanded by a fold expression
+  // outside. We should preserve the ContainsUnexpandedParameterPack flag here
+  // because CXXFoldExprs use it for the pattern.
+  // For example,
+  //
+  // []<class... Is>() { ([I = Is()]() {}, ...); }
+  //
+  // forgetting the flag will result in getPattern() of CXXFoldExpr returning
+  // null in terms of the inner lambda.
+  if (!LSICopy.ContainsUnexpandedParameterPack) {
+    llvm::SmallVector<UnexpandedParameterPack> UnexpandedPacks;
+    getSema().collectUnexpandedParameterPacksFromLambda(NewCallOperator,
+                                                        UnexpandedPacks);
+    // Should we call DiagnoseUnexpandedParameterPacks() instead?
+    LSICopy.ContainsUnexpandedParameterPack = !UnexpandedPacks.empty();
+  }
   return getSema().BuildLambdaExpr(E->getBeginLoc(), Body.get()->getEndLoc(),
                                    &LSICopy);
 }
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 35b2ffcefea35..31d85e1689af1 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -23,3 +23,60 @@ namespace PR41576 {
   }
   static_assert(f(3, 4) == 6); // expected-note {{instantiation}}
 }
+
+namespace PR85667 {
+
+template <class T>
+struct identity {
+  using type = T;
+};
+
+template <class = void> void f() {
+
+  static_assert([]<class... Is>(Is... x) {
+    return ([I(x)] {
+      return I;
+    }() + ...);
+  }(1, 2) == 3);
+
+  static_assert([]<class... Is>(Is... x) {
+    return ([](auto y = Is()) { return y + 1; } + ...);
+  }(0, 0, 0) == 3);
+
+  []<class... Is>() {
+    return ([]() noexcept(Is()) { return 0; }() + ...);
+  }.template operator()<int, int>();
+
+  static_assert(__is_same(decltype([]<class... Is>() {
+                            return ([]() -> decltype(Is()) { return {}; }(),
+                                    ...);
+                          }.template operator()<int, char>()),
+                          char));
+
+  []<class... Is>() {
+    return ([]<class... Ts>() -> decltype(Is()) { return Ts(); }() + ...);
+    // expected-error at -1 {{unexpanded parameter pack 'Ts'}}
+  }.template operator()<int, int>();
+
+  // Note that GCC and EDG reject this case currently.
+  // GCC says the fold expression "has no unexpanded parameter packs", while
+  // EDG says the constraint is not allowed on a non-template function.
+  // MSVC is happy with it.
+  []<class... Is>() {
+    ([]()
+       requires(Is())
+     {},
+     ...);
+  }.template operator()<bool, bool>();
+
+  // https://github.com/llvm/llvm-project/issues/56852
+  []<class... Is>(Is...) {
+    ([] {
+      using T = identity<Is>::type;
+    }(), ...);
+  }(1, 2);
+}
+
+template void f();
+
+}

>From e76fedd9fbe557f5c9a9f980a141c2ed31c24424 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 23 Mar 2024 19:39:37 +0800
Subject: [PATCH 02/10] Consider the captured pack variables - gh18873

---
 clang/docs/ReleaseNotes.rst                   |  4 ++--
 clang/lib/Sema/TreeTransform.h                |  4 ++++
 .../test/SemaTemplate/lambda-capture-pack.cpp | 21 +++++++++++++++++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2d11a4b8c092a..41529cbc9bd9a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -344,8 +344,8 @@ Bug Fixes to C++ Support
   when one of the function had more specialized templates.
   Fixes (`#82509 <https://github.com/llvm/llvm-project/issues/82509>`_)
   and (`#74494 <https://github.com/llvm/llvm-project/issues/74494>`_)
-- Fixed a crash where template parameter packs were not expanded correctly in a lambda being
-  used as a pattern of a folded expression. (#GH56852), (#GH85667)
+- Fixed a crash where template parameter packs were not expanded correctly in a lambda used
+  as the pattern of a folded expression. (#GH56852), (#GH85667), and partially fixes (#GH18873).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index f5a859c57034a..fb29e5111fc54 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13817,6 +13817,10 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
       continue;
     }
 
+    if (auto *PVD = dyn_cast<VarDecl>(CapturedVar);
+        PVD && !C->isPackExpansion())
+      LSI->ContainsUnexpandedParameterPack |= PVD->isParameterPack();
+
     // Capture the transformed variable.
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
                                  EllipsisLoc);
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 31d85e1689af1..8a81417b6daee 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -75,6 +75,27 @@ template <class = void> void f() {
       using T = identity<Is>::type;
     }(), ...);
   }(1, 2);
+
+  // https://github.com/llvm/llvm-project/issues/18873
+  [](auto ...y) {
+    ([y] { }, ...);
+  }();
+
+  [](auto ...x) {
+    ([&](auto ...y) {
+      // FIXME: This now hits the assertion with `PackIdx != -1 && "found declaration pack but not pack expanding"'
+      // in Sema::FindInstantiatedDecl.
+      // This is because the captured variable x has been expanded while transforming
+      // the outermost lambda call, but the expansion then gets holded off while transforming
+      // the folded expression. Then we would hit the assertion when instantiating the captured variable
+      // in TransformLambdaExpr.
+      // I think this is supposed to be ill-formed, but GCC and MSVC currently accept this.
+      // If x gets expanded with non-empty arguments, then GCC and MSVC will reject it - we probably
+      // miss a diagnostic for it.
+      // ([x, y] { }, ...);
+      ([x..., y] { }, ...);
+    })();
+  }();
 }
 
 template void f();

>From faf5d8710ba0177685cd65c6b81cf79b97223e4b Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sat, 23 Mar 2024 23:00:51 +0800
Subject: [PATCH 03/10] comments

---
 clang/lib/Sema/TreeTransform.h                  |  4 ++++
 clang/test/SemaTemplate/lambda-capture-pack.cpp | 12 ++++++------
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 0e2ea13d64d46..31592e6557f0d 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13786,6 +13786,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         }
         NewVDs.push_back(NewVD);
         getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
+        // The initializer might be expanded later. This may happen
+        // if the lambda is within a folded expression. 
         LSI->ContainsUnexpandedParameterPack |=
             Init.get()->containsUnexpandedParameterPack();
       }
@@ -13855,6 +13857,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
       continue;
     }
 
+    // The captured variable might be expanded later. This may happen
+    // if the lambda is within a folded expression. 
     if (auto *PVD = dyn_cast<VarDecl>(CapturedVar);
         PVD && !C->isPackExpansion())
       LSI->ContainsUnexpandedParameterPack |= PVD->isParameterPack();
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 8a81417b6daee..506902bc99dc6 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -83,15 +83,15 @@ template <class = void> void f() {
 
   [](auto ...x) {
     ([&](auto ...y) {
-      // FIXME: This now hits the assertion with `PackIdx != -1 && "found declaration pack but not pack expanding"'
+      // FIXME: This now hits assertion `PackIdx != -1 && "found declaration pack but not pack expanding"'
       // in Sema::FindInstantiatedDecl.
       // This is because the captured variable x has been expanded while transforming
-      // the outermost lambda call, but the expansion then gets holded off while transforming
-      // the folded expression. Then we would hit the assertion when instantiating the captured variable
-      // in TransformLambdaExpr.
+      // the outermost lambda call, but the expansion is held off while transforming
+      // the folded expression. Then, we would hit the assertion when instantiating the
+      // captured variable in TransformLambdaExpr.
       // I think this is supposed to be ill-formed, but GCC and MSVC currently accept this.
-      // If x gets expanded with non-empty arguments, then GCC and MSVC will reject it - we probably
-      // miss a diagnostic for it.
+      // However, if x gets expanded with non-empty arguments, then GCC and MSVC will reject it -
+      // we probably need a diagnostic for it.
       // ([x, y] { }, ...);
       ([x..., y] { }, ...);
     })();

>From c5765ec62f1d32b1cd817c8c78ef7b2d21bf296e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Sun, 24 Mar 2024 00:15:14 +0800
Subject: [PATCH 04/10] format

---
 clang/lib/Sema/TreeTransform.h | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 31592e6557f0d..e2ec9b7085737 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -13786,8 +13786,9 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         }
         NewVDs.push_back(NewVD);
         getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
-        // The initializer might be expanded later. This may happen
-        // if the lambda is within a folded expression. 
+        // If the lambda is written within a fold expression, the initializer
+        // may be expanded later. Preserve the ContainsUnexpandedParameterPack
+        // flag because CXXFoldExpr uses it for the pattern.
         LSI->ContainsUnexpandedParameterPack |=
             Init.get()->containsUnexpandedParameterPack();
       }
@@ -13848,6 +13849,17 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
       EllipsisLoc = C->getEllipsisLoc();
     }
+#if 0
+    else if (auto *PVD = cast<VarDecl>(C->getCapturedVar()); PVD->isParameterPack()) {
+      // If the lambda is written within a fold expression, the captured
+      // variable may be expanded later. Preserve the
+      // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
+      // pattern.
+      LSI->ContainsUnexpandedParameterPack |= true;
+      getSema().tryCaptureVariable(PVD, C->getLocation(), Kind, EllipsisLoc);
+      continue;
+    }
+#endif
 
     // Transform the captured variable.
     auto *CapturedVar = cast_or_null<ValueDecl>(
@@ -13857,11 +13869,15 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
       continue;
     }
 
-    // The captured variable might be expanded later. This may happen
-    // if the lambda is within a folded expression. 
+    // If the lambda is written within a fold expression, the captured
+    // variable may be expanded later. Preserve the
+    // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
+    // pattern.
+#if 1
     if (auto *PVD = dyn_cast<VarDecl>(CapturedVar);
         PVD && !C->isPackExpansion())
       LSI->ContainsUnexpandedParameterPack |= PVD->isParameterPack();
+#endif
 
     // Capture the transformed variable.
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
@@ -13984,12 +14000,10 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
                                     /*IsInstantiation*/ true);
   SavedContext.pop();
 
-  // The lambda may contain a pack that would be expanded by a fold expression
-  // outside. We should preserve the ContainsUnexpandedParameterPack flag here
-  // because CXXFoldExprs use it for the pattern.
-  // For example,
+  // The lambda function might contain a pack that would be expanded by a fold
+  // expression outside. For example,
   //
-  // []<class... Is>() { ([I = Is()]() {}, ...); }
+  // []<class... Is>() { ([](auto P = Is()) {}, ...); }
   //
   // forgetting the flag will result in getPattern() of CXXFoldExpr returning
   // null in terms of the inner lambda.
@@ -13997,7 +14011,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     llvm::SmallVector<UnexpandedParameterPack> UnexpandedPacks;
     getSema().collectUnexpandedParameterPacksFromLambda(NewCallOperator,
                                                         UnexpandedPacks);
-    // Should we call DiagnoseUnexpandedParameterPacks() instead?
+    // FIXME: Should we call DiagnoseUnexpandedParameterPacks() instead?
     LSICopy.ContainsUnexpandedParameterPack = !UnexpandedPacks.empty();
   }
   return getSema().BuildLambdaExpr(E->getBeginLoc(), Body.get()->getEndLoc(),

>From 6c2d311938839092f8fd1c5334673c6e35fe4651 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 3 May 2024 21:50:42 +0800
Subject: [PATCH 05/10] Retain the 'unexpanded' Decls

---
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  8 ++++++
 .../test/SemaTemplate/lambda-capture-pack.cpp | 26 +++++++++----------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 787a485e0b2f8..ac96449221384 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6221,6 +6221,14 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
           return cast<NamedDecl>(FD);
 
         int PackIdx = ArgumentPackSubstitutionIndex;
+        // FIXME: Move it elsewhere e.g. TreeTransform::TransformDecl.
+        // This is for #18873.
+        if (PackIdx == -1) {
+          LocalInstantiationScope LIS(*this, /*CombineWithOuterScope=*/false);
+          MultiLevelTemplateArgumentList FakeArgs;
+          FakeArgs.addOuterRetainedLevels(D->getTemplateDepth() + 1);
+          return cast_if_present<NamedDecl>(SubstDecl(D, CurContext, FakeArgs));
+        }
         assert(PackIdx != -1 &&
                "found declaration pack but not pack expanding");
         typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 506902bc99dc6..12cb2ba6cb86e 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -83,21 +83,21 @@ template <class = void> void f() {
 
   [](auto ...x) {
     ([&](auto ...y) {
-      // FIXME: This now hits assertion `PackIdx != -1 && "found declaration pack but not pack expanding"'
-      // in Sema::FindInstantiatedDecl.
-      // This is because the captured variable x has been expanded while transforming
-      // the outermost lambda call, but the expansion is held off while transforming
-      // the folded expression. Then, we would hit the assertion when instantiating the
-      // captured variable in TransformLambdaExpr.
-      // I think this is supposed to be ill-formed, but GCC and MSVC currently accept this.
-      // However, if x gets expanded with non-empty arguments, then GCC and MSVC will reject it -
-      // we probably need a diagnostic for it.
-      // ([x, y] { }, ...);
       ([x..., y] { }, ...);
-    })();
-  }();
+    })(1);
+  }(2, 'b');
+
+  [](auto ...x) {  // #outer
+    ([&](auto ...y) { // #inner
+      ([x, y] { }, ...);
+      // expected-error at -1 {{parameter pack 'y' that has a different length (4 vs. 3) from outer parameter packs}}
+      // expected-note-re@#inner {{function template specialization {{.*}} requested here}}
+      // expected-note-re@#outer {{function template specialization {{.*}} requested here}}
+      // expected-note-re@#instantiate-f {{function template specialization {{.*}} requested here}}
+    })('a', 'b', 'c');
+  }(0, 1, 2, 3);
 }
 
-template void f();
+template void f(); // #instantiate-f
 
 }

>From 1ac5ca73a1dd6084cae9ec54546f56fc18d7e0bc Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 3 May 2024 22:11:49 +0800
Subject: [PATCH 06/10] Update ReleaseNotes

---
 clang/docs/ReleaseNotes.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 81a6d67f42f7e..dd488f7ec17d1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -561,7 +561,7 @@ Bug Fixes to C++ Support
 - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()``
   function returns a large or negative value. Fixes (#GH89407).
 - Fixed a crash where template parameter packs were not expanded correctly in a lambda used
-  as the pattern of a folded expression. (#GH56852), (#GH85667), and partially fixed (#GH18873).
+  as the pattern of a folded expression. (#GH56852), (#GH85667) and (#GH18873).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

>From db36bc87d2a5bb64a918a53636f3f97524f883ad Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 8 May 2024 09:50:01 +0800
Subject: [PATCH 07/10] Cleanup & silence the unused-expression warning

---
 clang/lib/Sema/TreeTransform.h                 | 18 ++----------------
 .../test/SemaTemplate/lambda-capture-pack.cpp  | 12 +++++++++---
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ec14129259f83..e765d25ee606f 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14092,17 +14092,6 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
       EllipsisLoc = C->getEllipsisLoc();
     }
-#if 0
-    else if (auto *PVD = cast<VarDecl>(C->getCapturedVar()); PVD->isParameterPack()) {
-      // If the lambda is written within a fold expression, the captured
-      // variable may be expanded later. Preserve the
-      // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
-      // pattern.
-      LSI->ContainsUnexpandedParameterPack |= true;
-      getSema().tryCaptureVariable(PVD, C->getLocation(), Kind, EllipsisLoc);
-      continue;
-    }
-#endif
 
     // Transform the captured variable.
     auto *CapturedVar = cast_or_null<ValueDecl>(
@@ -14116,11 +14105,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     // variable may be expanded later. Preserve the
     // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
     // pattern.
-#if 1
-    if (auto *PVD = dyn_cast<VarDecl>(CapturedVar);
-        PVD && !C->isPackExpansion())
-      LSI->ContainsUnexpandedParameterPack |= PVD->isParameterPack();
-#endif
+    LSI->ContainsUnexpandedParameterPack |=
+        cast<VarDecl>(CapturedVar)->isParameterPack();
 
     // Capture the transformed variable.
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 12cb2ba6cb86e..7b22c724eb20e 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -78,18 +78,24 @@ template <class = void> void f() {
 
   // https://github.com/llvm/llvm-project/issues/18873
   [](auto ...y) {
-    ([y] { }, ...);
+    ([y] { }(), ...);
   }();
 
   [](auto ...x) {
     ([&](auto ...y) {
-      ([x..., y] { }, ...);
+      ([x..., y] { }(), ...);
     })(1);
   }(2, 'b');
 
+  [](auto ...x) {
+    ([&](auto ...y) {
+      ([x, y] { }(), ...);
+    })(1, 'a');
+  }(2, 'b');
+
   [](auto ...x) {  // #outer
     ([&](auto ...y) { // #inner
-      ([x, y] { }, ...);
+      ([x, y] { }(), ...);
       // expected-error at -1 {{parameter pack 'y' that has a different length (4 vs. 3) from outer parameter packs}}
       // expected-note-re@#inner {{function template specialization {{.*}} requested here}}
       // expected-note-re@#outer {{function template specialization {{.*}} requested here}}

>From 66acffb605aa724f59a2667bc3e542a23163e1e3 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 24 May 2024 13:33:20 +0800
Subject: [PATCH 08/10] Fix the CI

---
 clang/lib/Sema/TreeTransform.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index e765d25ee606f..3be68d8d251f3 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14105,8 +14105,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
     // variable may be expanded later. Preserve the
     // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
     // pattern.
-    LSI->ContainsUnexpandedParameterPack |=
-        cast<VarDecl>(CapturedVar)->isParameterPack();
+    if (auto *VD = dyn_cast<VarDecl>(CapturedVar); VD && !C->isPackExpansion())
+      LSI->ContainsUnexpandedParameterPack |= VD->isParameterPack();
 
     // Capture the transformed variable.
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,

>From 5285c104ed526311edc4a54842ca7844410d4780 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 24 May 2024 13:34:00 +0800
Subject: [PATCH 09/10] Fix typos

---
 clang/lib/Sema/SemaTemplateDeduction.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 0b6375001f532..2af3c44f49b3c 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -4296,7 +4296,7 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
 
   // Deduce an argument of type ParamType from an expression with index ArgIdx.
   auto DeduceCallArgument = [&](QualType ParamType, unsigned ArgIdx,
-                                bool ExplicitObjetArgument) {
+                                bool ExplicitObjectArgument) {
     // C++ [demp.deduct.call]p1: (DR1391)
     //   Template argument deduction is done by comparing each function template
     //   parameter that contains template-parameters that participate in
@@ -4304,7 +4304,7 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
     if (!hasDeducibleTemplateParameters(*this, FunctionTemplate, ParamType))
       return TemplateDeductionResult::Success;
 
-    if (ExplicitObjetArgument) {
+    if (ExplicitObjectArgument) {
       //   ... with the type of the corresponding argument
       return DeduceTemplateArgumentsFromCallArgument(
           *this, TemplateParams, FirstInnerIndex, ParamType, ObjectType,

>From b2d28364d1729b3f90b37905b91554b9a5d75423 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 18 Jul 2024 14:28:52 +0800
Subject: [PATCH 10/10] Fixup

---
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  8 -----
 clang/lib/Sema/TreeTransform.h                | 25 +++++++---------
 .../test/SemaTemplate/lambda-capture-pack.cpp | 29 +++++++++++++++----
 3 files changed, 34 insertions(+), 28 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 896e38d45326e..01432301633ed 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6113,14 +6113,6 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, NamedDecl *D,
           return cast<NamedDecl>(FD);
 
         int PackIdx = ArgumentPackSubstitutionIndex;
-        // FIXME: Move it elsewhere e.g. TreeTransform::TransformDecl.
-        // This is for #18873.
-        if (PackIdx == -1) {
-          LocalInstantiationScope LIS(*this, /*CombineWithOuterScope=*/false);
-          MultiLevelTemplateArgumentList FakeArgs;
-          FakeArgs.addOuterRetainedLevels(D->getTemplateDepth() + 1);
-          return cast_if_present<NamedDecl>(SubstDecl(D, CurContext, FakeArgs));
-        }
         assert(PackIdx != -1 &&
                "found declaration pack but not pack expanding");
         typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index ba074bc636361..1afe05d9ad524 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14443,9 +14443,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         }
         NewVDs.push_back(NewVD);
         getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
-        // If the lambda is written within a fold expression, the initializer
-        // may be expanded later. Preserve the ContainsUnexpandedParameterPack
-        // flag because CXXFoldExpr uses it for the pattern.
+        // The Init expression might be expanded by an outer fold expression.
         LSI->ContainsUnexpandedParameterPack |=
             Init.get()->containsUnexpandedParameterPack();
       }
@@ -14515,10 +14513,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
       continue;
     }
 
-    // If the lambda is written within a fold expression, the captured
-    // variable may be expanded later. Preserve the
-    // ContainsUnexpandedParameterPack flag because CXXFoldExpr uses it for the
-    // pattern.
+    // The captured pack might be expanded by an outer fold expression.
     if (auto *VD = dyn_cast<VarDecl>(CapturedVar); VD && !C->isPackExpansion())
       LSI->ContainsUnexpandedParameterPack |= VD->isParameterPack();
 
@@ -14574,6 +14569,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
     if (NewCallOpType.isNull())
       return ExprError();
+    LSI->ContainsUnexpandedParameterPack |=
+        NewCallOpType->containsUnexpandedParameterPack();
     NewCallOpTSI =
         NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
   }
@@ -14648,18 +14645,18 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
                                     /*IsInstantiation*/ true);
   SavedContext.pop();
 
-  // The lambda function might contain a pack that would be expanded by a fold
-  // expression outside. For example,
+  // Parts other than the capture e.g. the lambda body might still contain a
+  // pattern that an outer fold expression would expand.
   //
-  // []<class... Is>() { ([](auto P = Is()) {}, ...); }
-  //
-  // forgetting the flag will result in getPattern() of CXXFoldExpr returning
-  // null in terms of the inner lambda.
+  // We don't have a way to propagate up the ContainsUnexpandedParameterPack
+  // flag from a Stmt, so we have to revisit the lambda.
   if (!LSICopy.ContainsUnexpandedParameterPack) {
     llvm::SmallVector<UnexpandedParameterPack> UnexpandedPacks;
     getSema().collectUnexpandedParameterPacksFromLambda(NewCallOperator,
                                                         UnexpandedPacks);
-    // FIXME: Should we call DiagnoseUnexpandedParameterPacks() instead?
+    // FIXME: Should we call Sema::DiagnoseUnexpandedParameterPacks() instead?
+    // Unfortunately, that requires the LambdaScopeInfo to exist, which has been
+    // removed by ActOnFinishFunctionBody().
     LSICopy.ContainsUnexpandedParameterPack = !UnexpandedPacks.empty();
   }
   // Recompute the dependency of the lambda so that we can defer the lambda call
diff --git a/clang/test/SemaTemplate/lambda-capture-pack.cpp b/clang/test/SemaTemplate/lambda-capture-pack.cpp
index 7b22c724eb20e..6d2ab50827759 100644
--- a/clang/test/SemaTemplate/lambda-capture-pack.cpp
+++ b/clang/test/SemaTemplate/lambda-capture-pack.cpp
@@ -76,7 +76,6 @@ template <class = void> void f() {
     }(), ...);
   }(1, 2);
 
-  // https://github.com/llvm/llvm-project/issues/18873
   [](auto ...y) {
     ([y] { }(), ...);
   }();
@@ -87,11 +86,28 @@ template <class = void> void f() {
     })(1);
   }(2, 'b');
 
-  [](auto ...x) {
-    ([&](auto ...y) {
-      ([x, y] { }(), ...);
-    })(1, 'a');
-  }(2, 'b');
+#if 0
+  // https://github.com/llvm/llvm-project/issues/18873
+  [](auto ...x) { // #1
+    ([&](auto ...y) {  // #2
+      ([x, y] { }(), ...); // #3
+    })(1, 'a');  // #4
+  }(2, 'b');  // #5
+
+  // We run into another crash for the above lambda because of the absence of a
+  // mechanism that rebuilds an unexpanded pack from an expanded Decls.
+  //
+  // Basically, this happens after `x` at #1 being expanded when the template
+  // arguments at #5, deduced as <int, char>, are ready. When we want to
+  // instantiate the body of #1, we first instantiate the CallExpr at #4, which
+  // boils down to the lambda's instantiation at #2. To that end, we have to
+  // instantiate the body of it, which turns out to be #3. #3 is a CXXFoldExpr,
+  // and we immediately have to hold off on the expansion because we don't have
+  // corresponding template arguments for it. Therefore, we want to rebuild a
+  // CXXFoldExpr, which requires another pattern transformation of the lambda
+  // inside #3. Then we need to find an unexpanded form of such a Decl of x at
+  // the time of transforming the capture, which is impossible because the
+  // instantiated form has been expanded at #1!
 
   [](auto ...x) {  // #outer
     ([&](auto ...y) { // #inner
@@ -102,6 +118,7 @@ template <class = void> void f() {
       // expected-note-re@#instantiate-f {{function template specialization {{.*}} requested here}}
     })('a', 'b', 'c');
   }(0, 1, 2, 3);
+#endif
 }
 
 template void f(); // #instantiate-f



More information about the cfe-commits mailing list