[Clang] Don't assert on substituted-but-yet-expanded packs for nested lambdas

[zyn0217]

Nested lambdas could refer to outer packs that would be expanded by a larger CXXFoldExpr, in which case that reference could happen to be a full expression containing intermediate types/expressions, e.g. SubstTemplateTypeParmPackType/FunctionParmPackExpr. They are designated as "UnexpandedPack" dependencies but don't introduce new packs anyway.

This also handles a missed case for VarDecls, where the flag of ContainsUnexpandedPack was not propagated up to the surrounding lambda.

Fixes #112352

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

Nested lambdas could refer to outer packs that would be expanded by a larger CXXFoldExpr, in which case that reference could happen to be a full expression containing intermediate types/expressions, e.g. SubstTemplateTypeParmPackType/FunctionParmPackExpr. They are designated as "UnexpandedPack" dependencies but don't introduce new packs anyway.

Fixes #112352

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1-1)
• (modified) clang/lib/Sema/SemaTemplateVariadic.cpp (+34-15)
• (modified) clang/test/SemaCXX/lambda-pack-expansion.cpp (+40)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b430b2b0ee3187..d071c480f36dc7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -486,7 +486,7 @@ Bug Fixes to C++ Support
 - 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)
+- A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361), (#GH112352)
 - Fixed a crash in the typo correction of an invalid CTAD guide. (#GH107887)
 - Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter
   pack. (#GH63819), (#GH107560)
diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp
index 19bd4547665835..151b328872bd75 100644
--- a/clang/lib/Sema/SemaTemplateVariadic.cpp
+++ b/clang/lib/Sema/SemaTemplateVariadic.cpp
@@ -41,7 +41,7 @@ namespace {
     unsigned DepthLimit = (unsigned)-1;
 
 #ifndef NDEBUG
-    bool ContainsFunctionParmPackExpr = false;
+    bool ContainsIntermediatePacks = false;
 #endif
 
     void addUnexpanded(NamedDecl *ND, SourceLocation Loc = SourceLocation()) {
@@ -114,6 +114,11 @@ namespace {
           addUnexpanded(TTP);
       }
 
+#ifndef NDEBUG
+      ContainsIntermediatePacks |=
+          (bool)Template.getAsSubstTemplateTemplateParmPack();
+#endif
+
       return inherited::TraverseTemplateName(Template);
     }
 
@@ -297,13 +302,28 @@ namespace {
 
 #ifndef NDEBUG
     bool TraverseFunctionParmPackExpr(FunctionParmPackExpr *) {
-      ContainsFunctionParmPackExpr = true;
+      ContainsIntermediatePacks = true;
+      return true;
+    }
+
+    bool TraverseSubstNonTypeTemplateParmPackExpr(
+        SubstNonTypeTemplateParmPackExpr *) {
+      ContainsIntermediatePacks = true;
+      return true;
+    }
+
+    bool VisitSubstTemplateTypeParmPackType(SubstTemplateTypeParmPackType *) {
+      ContainsIntermediatePacks = true;
       return true;
     }
 
-    bool containsFunctionParmPackExpr() const {
-      return ContainsFunctionParmPackExpr;
+    bool
+    VisitSubstTemplateTypeParmPackTypeLoc(SubstTemplateTypeParmPackTypeLoc) {
+      ContainsIntermediatePacks = true;
+      return true;
     }
+
+    bool containsIntermediatePacks() const { return ContainsIntermediatePacks; }
 #endif
   };
 }
@@ -439,21 +459,20 @@ bool Sema::DiagnoseUnexpandedParameterPack(Expr *E,
   if (!E->containsUnexpandedParameterPack())
     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 Visitor(Unexpanded);
   Visitor.TraverseStmt(E);
-  assert((!Unexpanded.empty() || Visitor.containsFunctionParmPackExpr()) &&
+#ifndef NDEBUG
+  // The expression might contain a type/subexpression that has been substituted
+  // but has the expansion held off, e.g. a FunctionParmPackExpr which a larger
+  // CXXFoldExpr would expand. It's only possible when expanding a lambda as a
+  // pattern of a fold expression, so don't fire on an empty result in that
+  // case.
+  bool LambdaReferencingOuterPacks =
+      getEnclosingLambdaOrBlock() && Visitor.containsIntermediatePacks();
+  assert((!Unexpanded.empty() || LambdaReferencingOuterPacks) &&
          "Unable to find unexpanded parameter packs");
+#endif
   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 0e60ecd8756600..7f7dd3bb095d57 100644
--- a/clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ b/clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -94,3 +94,43 @@ template <typename... Ts> void g2(Ts... p1s) {
 void f1() { g(); }
 
 } // namespace GH61460
+
+namespace GH112352 {
+
+template <class>
+constexpr bool foo = false;
+
+template <int>
+constexpr bool bar = false;
+
+template <template<class> class>
+constexpr bool baz = false;
+
+struct S {
+  template <typename... Types, int... Values> void foldExpr1() {
+    (void)[]<int... Is> {
+      ([] {
+        Is;
+        foo<Types>;
+        bar<Values>;
+      } &&
+       ...);
+    };
+  }
+
+  template <template<class> class... TTPs> void foldExpr2() {
+    (void)[]<int... Is> {
+      ([] {
+        Is;
+        baz<TTPs>;
+      } && ...);
+    };
+  }
+};
+
+void use() {
+  S().foldExpr1();
+  S().foldExpr2();
+}
+
+} // namespace GH112352

[cor3ntin]

The more I see that sort of things, the more @zygoloid 's idea of tracking unexpanded packs lexically seems compelling.

we probably should have have a separate function for "does this declaration contains an unexpanded pack"

[zyn0217]

I'm not sure if I understand "tracking unexpanded packs lexically". Do you have a link to that context? It sounds like we need a new bit to track that in Decls, despite that flag being accessible through their QualTypes' dependencies for the status quo. In the latter regard, it looks like we just need a shortcut method for it?

[cor3ntin]

Haha, this is mostly a "gosh, this is getting unwieldy, i wonder if there is a better way", rather than a request for a change. I was referring to this #9395 (comment)

[cor3ntin]

LGTM