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

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

<details>
<summary>Changes</summary>

`FunctionParmPackExpr`s are peculiar in that they have to be of unexpanded dependency while they don't introduce any unexpanded packs. So this patch rules them out in the non-empty pack assertion in `DiagnoseUnexpandedParameterPack()`

There was a fix #<!-- -->69224, but that turned out to be insufficient.

I also moved the separate tests to a pre-existing file.

Fixes https://github.com/llvm/llvm-project/issues/86361

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


4 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1-1) 
- (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+24-9) 
- (modified) clang/test/SemaCXX/lambda-pack-expansion.cpp (+22) 
- (removed) clang/test/SemaCXX/pr61460.cpp (-13) 


``````````diff
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'}}
-}

``````````

</details>


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