[clang] [Clang] Don't assert non-empty packs for FunctionParmPackExprs (PR #107561)

[Younan Zhang via cfe-commits]

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

>From 92a30ff72fd922df3c29ac31fc2c7d46df1ff84e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 6 Sep 2024 18:35:45 +0800
Subject: [PATCH 1/3] [Clang] Don't assert non-empty packs for
 FunctionParmPackExprs

---
 clang/docs/ReleaseNotes.rst                  |  2 +-
 clang/lib/Sema/SemaTemplateVariadic.cpp      | 33 ++++++++++++++------
 clang/test/SemaCXX/lambda-pack-expansion.cpp | 22 +++++++++++++
 clang/test/SemaCXX/pr61460.cpp               | 13 --------
 4 files changed, 47 insertions(+), 23 deletions(-)
 delete mode 100644 clang/test/SemaCXX/pr61460.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a2e91fd648cce2..3b63b78521e351 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -368,8 +368,8 @@ Bug Fixes to C++ Support
 - Fixed a bug in the substitution of empty pack indexing types. (#GH105903)
 - Clang no longer tries to capture non-odr used default arguments of template parameters of generic lambdas (#GH107048)
 - Fixed a bug where defaulted comparison operators would remove ``const`` from base classes. (#GH102588)
-
 - Fix a crash when using ``source_location`` in the trailing return type of a lambda expression. (#GH67134)
+- A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index bcd31c98871e22..0a9fd9ab8a5fe1 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,6 +38,7 @@ namespace {
 
     bool InLambda = false;
     unsigned DepthLimit = (unsigned)-1;
+    bool ContainsFunctionParmPackExpr = false;
 
     void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {
       if (auto *VD = dyn_cast<VarDecl>(ND)) {
@@ -280,6 +281,15 @@ namespace {
 
       return inherited::TraverseLambdaCapture(Lambda, C, Init);
     }
+
+    bool TraverseFunctionParmPackExpr(FunctionParmPackExpr *) {
+      ContainsFunctionParmPackExpr = true;
+      return true;
+    }
+
+    bool containsFunctionParmPackExpr() const {
+      return ContainsFunctionParmPackExpr;
+    }
   };
 }
 
@@ -414,16 +424,21 @@ bool Sema::DiagnoseUnexpandedParameterPack(Expr *E,
   if (!E->containsUnexpandedParameterPack())
     return false;
 
-  // CollectUnexpandedParameterPacksVisitor does not expect to see a
-  // FunctionParmPackExpr, but diagnosing unexpected parameter packs may still
-  // see such an expression in a lambda body.
-  // We'll bail out early in this case to avoid triggering an assertion.
-  if (isa<FunctionParmPackExpr>(E) && getEnclosingLambda())
-    return false;
-
+  // FunctionParmPackExprs are special:
+  //
+  // 1) they're used to model DeclRefExprs to packs that have been expanded but
+  // had that expansion held off in the process of transformation.
+  //
+  // 2) they always have the unexpanded dependencies but don't introduce new
+  // unexpanded packs.
+  //
+  // We might encounter a FunctionParmPackExpr being a full expression, which a
+  // larger CXXFoldExpr would expand.
   SmallVector<UnexpandedParameterPack, 2> Unexpanded;
-  CollectUnexpandedParameterPacksVisitor(Unexpanded).TraverseStmt(E);
-  assert(!Unexpanded.empty() && "Unable to find unexpanded parameter packs");
+  CollectUnexpandedParameterPacksVisitor Visitor(Unexpanded);
+  Visitor.TraverseStmt(E);
+  assert((!Unexpanded.empty() || Visitor.containsFunctionParmPackExpr()) &&
+         "Unable to find unexpanded parameter packs");
   return DiagnoseUnexpandedParameterPacks(E->getBeginLoc(), UPPC, Unexpanded);
 }
 
diff --git a/clang/test/SemaCXX/lambda-pack-expansion.cpp b/clang/test/SemaCXX/lambda-pack-expansion.cpp
index 77b2e244753a94..5c702e1166a6bc 100644
--- a/clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ b/clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -68,3 +68,25 @@ void f() {
 }
 
 }
+
+namespace GH61460 {
+
+template<typename... Ts>
+void f1(Ts... ts);
+
+template <typename... Ts> void g(Ts... p1s) {
+  (void)[&](auto... p2s) {
+    (
+        [&] {
+          p1s;
+          f1(p1s);
+          sizeof(p1s);
+          p2s;
+        },
+        ...);
+  };
+}
+
+void f1() { g(); }
+
+} // namespace GH61460
diff --git a/clang/test/SemaCXX/pr61460.cpp b/clang/test/SemaCXX/pr61460.cpp
deleted file mode 100644
index 471b1b39d23c2b..00000000000000
--- a/clang/test/SemaCXX/pr61460.cpp
+++ /dev/null
@@ -1,13 +0,0 @@
-// RUN: %clang_cc1 -std=c++17 %s -fsyntax-only -verify
-
-template <typename... Ts> void g(Ts... p1s) {
-  (void)[&](auto... p2s) { ([&] { p1s; p2s; }, ...); };
-}
-
-void f1() {
-  g();
-}
-
-template <typename... Ts> void g2(Ts... p1s) {
-  (void)[&](auto... p2s) { [&] { p1s; p2s; }; }; // expected-error {{expression contains unexpanded parameter pack 'p2s'}}
-}

>From 225aae989764d4e445a69f4c2bb8c2a18ae1e16b Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 6 Sep 2024 22:36:34 +0800
Subject: [PATCH 2/3] Recover the test

---
 clang/test/SemaCXX/lambda-pack-expansion.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/test/SemaCXX/lambda-pack-expansion.cpp b/clang/test/SemaCXX/lambda-pack-expansion.cpp
index 5c702e1166a6bc..0e60ecd8756600 100644
--- a/clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ b/clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -87,6 +87,10 @@ template <typename... Ts> void g(Ts... p1s) {
   };
 }
 
+template <typename... Ts> void g2(Ts... p1s) {
+  (void)[&](auto... p2s) { [&] { p1s; p2s; }; }; // expected-error {{unexpanded parameter pack 'p2s'}}
+}
+
 void f1() { g(); }
 
 } // namespace GH61460

>From b14f454a596470c8e4e665d765880af179690c75 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 9 Sep 2024 14:16:10 +0800
Subject: [PATCH 3/3] Only traverse FunctionParmPackExprs when assert is
 available

---
 clang/lib/Sema/SemaTemplateVariadic.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 0a9fd9ab8a5fe1..40522a07f6339c 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -38,7 +38,10 @@ namespace {
 
     bool InLambda = false;
     unsigned DepthLimit = (unsigned)-1;
+
+#ifndef NDEBUG
     bool ContainsFunctionParmPackExpr = false;
+#endif
 
     void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {
       if (auto *VD = dyn_cast<VarDecl>(ND)) {
@@ -282,6 +285,7 @@ namespace {
       return inherited::TraverseLambdaCapture(Lambda, C, Init);
     }
 
+#ifndef NDEBUG
     bool TraverseFunctionParmPackExpr(FunctionParmPackExpr *) {
       ContainsFunctionParmPackExpr = true;
       return true;
@@ -290,6 +294,7 @@ namespace {
     bool containsFunctionParmPackExpr() const {
       return ContainsFunctionParmPackExpr;
     }
+#endif
   };
 }