[Clang][SemaCXX] Fix bug where unexpanded lambda captures where assumed to have size 1

[MitalAshok]

Fixes #63677

[llvmbot]

@llvm/pr-subscribers-clang

Author: Mital Ashok (MitalAshok)

Changes

Fixes #63677

---

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

4 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+2-1)
• (modified) clang/lib/Sema/TreeTransform.h (+3-3)
• (modified) clang/test/SemaCXX/lambda-pack-expansion.cpp (+27)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 3c2e0282d1c72..2f4158ac36ac2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -170,6 +170,7 @@ Bug Fixes to C++ Support
 - Fixed a failed assertion when checking invalid delete operator declaration. (#GH96191)
 - Fix a crash when checking destructor reference with an invalid initializer. (#GH97230)
 - Clang now correctly parses potentially declarative nested-name-specifiers in pointer-to-member declarators.
+- Fix init-capture packs having a size of one before being instantiated. (#GH63677)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 8995d461362d7..de470739ab78e 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1447,7 +1447,8 @@ namespace {
     }
 
     void transformedLocalDecl(Decl *Old, ArrayRef<Decl *> NewDecls) {
-      if (Old->isParameterPack()) {
+      if (Old->isParameterPack() &&
+          (NewDecls.size() != 1 || !NewDecls.front()->isParameterPack())) {
         SemaRef.CurrentInstantiationScope->MakeInstantiatedLocalArgPack(Old);
         for (auto *New : NewDecls)
           SemaRef.CurrentInstantiationScope->InstantiatedLocalPackArg(
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 8d3e1edf7a45d..540e1e0cb8df0 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14340,14 +14340,14 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
               OldVD->getInit()->getSourceRange(), Unexpanded, Expand,
               RetainExpansion, NumExpansions))
         return ExprError();
+      assert(!RetainExpansion && "Should not need to retain expansion after a "
+                                 "capture since it cannot be extended");
       if (Expand) {
         for (unsigned I = 0; I != *NumExpansions; ++I) {
           Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
           SubstInitCapture(SourceLocation(), std::nullopt);
         }
-      }
-      if (!Expand || RetainExpansion) {
-        ForgetPartiallySubstitutedPackRAII Forget(getDerived());
+      } else {
         SubstInitCapture(ExpansionTL.getEllipsisLoc(), NumExpansions);
         Result.EllipsisLoc = ExpansionTL.getEllipsisLoc();
       }
diff --git a/clang/test/SemaCXX/lambda-pack-expansion.cpp b/clang/test/SemaCXX/lambda-pack-expansion.cpp
index 221d1d01a06ae..77b2e244753a9 100644
--- a/clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ b/clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -41,3 +41,30 @@ int h(Ts... ts) {
 }
 
 }
+
+namespace GH63677 {
+
+template<typename>
+void f() {
+  []<typename... Ts>() -> void {
+    [...us = Ts{}]{
+      (Ts(us), ...);
+    };
+  }.template operator()<int, int>();
+}
+
+template void f<int>();
+
+template <class>
+inline constexpr auto fun =
+  []<class... Ts>(Ts... ts) {
+    return [... us = (Ts&&) ts]<class Fun>(Fun&& fn) mutable {
+      return static_cast<Fun&&>(fn)(static_cast<Ts&&>(us)...);
+    };
+  };
+
+void f() {
+  [[maybe_unused]] auto s = fun<int>(1, 2, 3, 4);
+}
+
+}

[MitalAshok]

Revived from https://reviews.llvm.org/D154716

This probably needs a fresh review, I've just rebased onto main.

The problem still exists on main: https://godbolt.org/z/vPTb4qEnd

CC @cor3ntin @shafik (as people who reviewed on the phabricator)

[zyn0217]

I remembered @LYP951018 shared me with a case involving PackIndexingExprs that was incorrectly rejected due to the mishandling of PackExpansionExprs:

namespace init_capture_pack {

void init_capture() {
  auto L = [](auto... x) {
    return [x...](auto... y) {
      return [... w = y]() {
        return w...[3];
      };
    };
  };
  static_assert(L()(0, 1, 2, 3)() == 3);
}

I have prepared for a fix on my branch zyn0217@72fc2fc, which somehow relies on this "the unexpanded pack should have size 1" feature. I applied this PR locally only to find we ran into a crash with the case afterward.

bool clang::Sema::DiagnoseUnexpandedParameterPack(Expr *, UnexpandedParameterPackContext): Assertion `!Unexpanded.empty() && "Unable to find unexpanded parameter packs"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: ../llvm-project-Build/BuildDebug/bin/clang -fsyntax-only -fno-crash-diagnostics -std=c++20 /clangd-test/pack-indexing.cpp -DCRASH
1.      /clangd-test/pack-indexing.cpp:11:19: current parser token ')'
2.      /clangd-test/pack-indexing.cpp:1:1: parsing namespace 'init_capture_pack'
3.      /clangd-test/pack-indexing.cpp:3:21: parsing function body 'init_capture_pack::init_capture'
4.      /clangd-test/pack-indexing.cpp:3:21: in compound statement ('{}')
5.      /clangd-test/pack-indexing.cpp:4:12: instantiating function definition 'init_capture_pack::init_capture()::(anonymous class)::operator()<>'

#11 0x00007f68109c9dad clang::Sema::DiagnoseUnexpandedParameterPack(clang::Expr*, clang::Sema::UnexpandedParameterPackContext) /repo/llvm-project/clang/lib/Sema/SemaTemplateVariadic.cpp:427:43
#12 0x00007f681049bdd1 clang::Sema::BuildReturnStmt(clang::SourceLocation, clang::Expr*, bool) /repo/llvm-project/clang/lib/Sema/SemaStmt.cpp:3782:7
#13 0x00007f681091c6a3 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::RebuildReturnStmt(clang::SourceLocation, clang::Expr*) /repo/llvm-project/clang/lib/Sema/TreeTransform.h:1501:22
#14 0x00007f68109045d7 clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformReturnStmt(clang::ReturnStmt*) /repo/llvm-project/clang/lib/Sema/TreeTransform.h:8288:23
#15 0x00007f68108be03e clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformStmt(clang::Stmt*, clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::StmtDiscardKind) /repo/llvm-project-Build/BuildDebug/tools/clang/include/clang/AST/StmtNodes.inc:926:1
#16 0x00007f68108dabbb clang::TreeTransform<(anonymous namespace)::TemplateInstantiator>::TransformCompoundStmt(clang::CompoundStmt*, bool) /repo/llvm-project/clang/lib/Sema/TreeTransform.h:7859:38

[zyn0217]

Sorry I was wrong. I mixed up my patch along with this fix so I reapplied it to a clean branch locally, and this could also fix the above case. With this patch, we don't need to bother to handling PackExpansionExpr in the transform pass of PackIndexingExpr, which is great!

[zyn0217]

Thanks!

[zyn0217]

@MitalAshok Can you rebase the patch so that I can help you merge it? thanks!