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

[via cfe-commits]

Author: Younan Zhang
Date: 2024-09-09T15:09:43+08:00
New Revision: 8549b324bc1f450f4477f46f18db67439dbf6d75

URL: https://github.com/llvm/llvm-project/commit/8549b324bc1f450f4477f46f18db67439dbf6d75
DIFF: https://github.com/llvm/llvm-project/commit/8549b324bc1f450f4477f46f18db67439dbf6d75.diff

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

`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

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaTemplateVariadic.cpp
    clang/test/SemaCXX/lambda-pack-expansion.cpp

Removed: 
    clang/test/SemaCXX/pr61460.cpp


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a300829cf0e32c..07f3544e2324e3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -379,8 +379,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..40522a07f6339c 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -39,6 +39,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)) {
         // For now, the only problematic case is a generic lambda's templated
@@ -280,6 +284,17 @@ namespace {
 
       return inherited::TraverseLambdaCapture(Lambda, C, Init);
     }
+
+#ifndef NDEBUG
+    bool TraverseFunctionParmPackExpr(FunctionParmPackExpr *) {
+      ContainsFunctionParmPackExpr = true;
+      return true;
+    }
+
+    bool containsFunctionParmPackExpr() const {
+      return ContainsFunctionParmPackExpr;
+    }
+#endif
   };
 }
 
@@ -414,16 +429,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..0e60ecd8756600 100644
--- a/clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ b/clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -68,3 +68,29 @@ 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;
+        },
+        ...);
+  };
+}
+
+template <typename... Ts> void g2(Ts... p1s) {
+  (void)[&](auto... p2s) { [&] { p1s; p2s; }; }; // expected-error {{unexpanded parameter pack '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'}}
-}