[Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (llvm#86265)

The lambda `ContainsUnexpandedParameterPack` flag is used for the
expressions' dependency computing and is therefore essential for pack
expansion. We previously lost the flag's preservation during the
lambda's transform, which caused some issues, e.g. a fold expression
couldn't properly expand inside a template.

This patch alleviates the issue by retaining the flag in more scenarios.
Note that we still have problems with constraints involving packs
regarding lambdas, and dealing with that would take more effort, and
we'd like to fix them in the future.

Fixes llvm#56852
Fixes llvm#85667
Mitigates llvm#99877 because the
attributes were not handled in this patch.

---------

Co-authored-by: Ilya Biryukov <[email protected]>
Co-authored-by: cor3ntin <[email protected]>

Author: 3 people

clang/docs/ReleaseNotes.rst

@@ -186,6 +186,8 @@ Bug Fixes to C++ Support
   substitutions in concepts, so it doesn't incorrectly complain of missing
   module imports in those situations. (#GH60336)
 - Fix init-capture packs having a size of one before being instantiated. (#GH63677)
+- Clang now preserves the unexpanded flag in a lambda transform used for pack expansion. (#GH56852), (#GH85667),
+  (#GH99877).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaLambda.cpp

@@ -1021,6 +1021,8 @@ void Sema::CompleteLambdaCallOperator(
       getGenericLambdaTemplateParameterList(LSI, *this);
 
   DeclContext *DC = Method->getLexicalDeclContext();
+  // DeclContext::addDecl() assumes that the DeclContext we're adding to is the
+  // lexical context of the Method. Do so.
   Method->setLexicalDeclContext(LSI->Lambda);
   if (TemplateParams) {
     FunctionTemplateDecl *TemplateMethod =
@@ -1105,6 +1107,8 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
 
   CXXMethodDecl *Method = CreateLambdaCallOperator(Intro.Range, Class);
   LSI->CallOperator = Method;
+  // Temporarily set the lexical declaration context to the current
+  // context, so that the Scope stack matches the lexical nesting.
   Method->setLexicalDeclContext(CurContext);
 
   PushDeclContext(CurScope, Method);

clang/lib/Sema/SemaTemplateInstantiate.cpp

@@ -39,6 +39,7 @@
 #include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/TimeProfiler.h"
 #include <optional>
 
@@ -1657,11 +1658,12 @@ namespace {
       LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
       Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);
 
-      ExprResult Result = inherited::TransformLambdaExpr(E);
-      if (Result.isInvalid())
-        return Result;
+      return inherited::TransformLambdaExpr(E);
+    }
 
-      CXXMethodDecl *MD = Result.getAs<LambdaExpr>()->getCallOperator();
+    ExprResult RebuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
+                                 LambdaScopeInfo *LSI) {
+      CXXMethodDecl *MD = LSI->CallOperator;
       for (ParmVarDecl *PVD : MD->parameters()) {
         assert(PVD && "null in a parameter list");
         if (!PVD->hasDefaultArg())
@@ -1680,8 +1682,7 @@ namespace {
             PVD->setDefaultArg(ErrorResult.get());
         }
       }
-
-      return Result;
+      return inherited::RebuildLambdaExpr(StartLoc, EndLoc, LSI);
     }
 
     StmtResult TransformLambdaBody(LambdaExpr *E, Stmt *Body) {
@@ -1694,11 +1695,8 @@ namespace {
       // `true` to temporarily fix this issue.
       // FIXME: This temporary fix can be removed after fully implementing
       // p0588r1.
-      bool Prev = EvaluateConstraints;
-      EvaluateConstraints = true;
-      StmtResult Stmt = inherited::TransformLambdaBody(E, Body);
-      EvaluateConstraints = Prev;
-      return Stmt;
+      llvm::SaveAndRestore _(EvaluateConstraints, true);
+      return inherited::TransformLambdaBody(E, Body);
     }
 
     ExprResult TransformRequiresExpr(RequiresExpr *E) {

clang/lib/Sema/TreeTransform.h

@@ -4028,6 +4028,20 @@ class TreeTransform {
                                       NumExpansions);
   }
 
+  ExprResult RebuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc,
+                               LambdaScopeInfo *LSI) {
+    for (ParmVarDecl *PVD : LSI->CallOperator->parameters()) {
+      if (Expr *Init = PVD->getInit())
+        LSI->ContainsUnexpandedParameterPack |=
+            Init->containsUnexpandedParameterPack();
+      else if (PVD->hasUninstantiatedDefaultArg())
+        LSI->ContainsUnexpandedParameterPack |=
+            PVD->getUninstantiatedDefaultArg()
+                ->containsUnexpandedParameterPack();
+    }
+    return getSema().BuildLambdaExpr(StartLoc, EndLoc, LSI);
+  }
+
   /// Build an empty C++1z fold-expression with the given operator.
   ///
   /// By default, produces the fallback value for the fold-expression, or
@@ -8284,6 +8298,7 @@ StmtResult
 TreeTransform<Derived>::TransformDeclStmt(DeclStmt *S) {
   bool DeclChanged = false;
   SmallVector<Decl *, 4> Decls;
+  LambdaScopeInfo *LSI = getSema().getCurLambda();
   for (auto *D : S->decls()) {
     Decl *Transformed = getDerived().TransformDefinition(D->getLocation(), D);
     if (!Transformed)
@@ -8292,6 +8307,15 @@ TreeTransform<Derived>::TransformDeclStmt(DeclStmt *S) {
     if (Transformed != D)
       DeclChanged = true;
 
+    if (LSI && isa<TypeDecl>(Transformed))
+      LSI->ContainsUnexpandedParameterPack |=
+          getSema()
+              .getASTContext()
+              .getTypeDeclType(cast<TypeDecl>(Transformed))
+              .getCanonicalType()
+              .getTypePtr()
+              ->containsUnexpandedParameterPack();
+
     Decls.push_back(Transformed);
   }
 
@@ -14523,7 +14547,6 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
   CXXMethodDecl *NewCallOperator =
       getSema().CreateLambdaCallOperator(E->getIntroducerRange(), Class);
-  NewCallOperator->setLexicalDeclContext(getSema().CurContext);
 
   // Enter the scope of the lambda.
   getSema().buildLambdaScope(LSI, NewCallOperator, E->getIntroducerRange(),
@@ -14591,6 +14614,13 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
         }
         NewVDs.push_back(NewVD);
         getSema().addInitCapture(LSI, NewVD, C->getCaptureKind() == LCK_ByRef);
+        // Cases we want to tackle:
+        //   ([C(Pack)] {}, ...)
+        // But rule out cases e.g.
+        //    [...C = Pack()] {}
+        if (NewC.EllipsisLoc.isInvalid())
+          LSI->ContainsUnexpandedParameterPack |=
+              Init.get()->containsUnexpandedParameterPack();
       }
 
       if (Invalid)
@@ -14658,6 +14688,11 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
       continue;
     }
 
+    // This is not an init-capture; however it contains an unexpanded pack e.g.
+    //  ([Pack] {}(), ...)
+    if (auto *VD = dyn_cast<VarDecl>(CapturedVar); VD && !C->isPackExpansion())
+      LSI->ContainsUnexpandedParameterPack |= VD->isParameterPack();
+
     // Capture the transformed variable.
     getSema().tryCaptureVariable(CapturedVar, C->getLocation(), Kind,
                                  EllipsisLoc);
@@ -14669,9 +14704,12 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   auto TPL = getDerived().TransformTemplateParameterList(
       E->getTemplateParameterList());
   LSI->GLTemplateParameterList = TPL;
-  if (TPL)
+  if (TPL) {
     getSema().AddTemplateParametersToLambdaCallOperator(NewCallOperator, Class,
                                                         TPL);
+    LSI->ContainsUnexpandedParameterPack |=
+        TPL->containsUnexpandedParameterPack();
+  }
 
   // Transform the type of the original lambda's call operator.
   // The transformation MUST be done in the CurrentInstantiationScope since
@@ -14710,6 +14748,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
 
     if (NewCallOpType.isNull())
       return ExprError();
+    LSI->ContainsUnexpandedParameterPack |=
+        NewCallOpType->containsUnexpandedParameterPack();
     NewCallOpTSI =
         NewCallOpTLBuilder.getTypeSourceInfo(getSema().Context, NewCallOpType);
   }
@@ -14824,8 +14864,8 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
   Class->setTypeForDecl(nullptr);
   getSema().Context.getTypeDeclType(Class);
 
-  return getSema().BuildLambdaExpr(E->getBeginLoc(), Body.get()->getEndLoc(),
-                                   &LSICopy);
+  return getDerived().RebuildLambdaExpr(E->getBeginLoc(),
+                                        Body.get()->getEndLoc(), &LSICopy);
 }
 
 template<typename Derived>

clang/test/SemaCXX/fold_lambda_with_variadics.cpp

@@ -0,0 +1,181 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
+
+namespace GH85667 {
+
+template <class T>
+struct identity {
+  using type = T;
+};
+
+template <class = void> void f() {
+
+  static_assert([]<class... Is>(Is... x) {
+    return ([I(x)] {
+      return I;
+    }() + ...);
+  }(1, 2) == 3);
+
+  []<class... Is>(Is... x) {
+    return ([](auto y = Is()) { return y + 1; }() + ...); // expected-error {{no matching function}}                     \
+                                                          // expected-note {{couldn't infer template argument 'y:auto'}} \
+                                                          // expected-note@-1 {{requested here}}
+                                                          // expected-note@#instantiate-f {{requested here}}
+  }(1);
+
+  []<class... Is>() {
+    ([]<class = Is>(Is)
+       noexcept(bool(Is()))
+     {}(Is()),
+     ...);
+  }.template operator()<char, int, float>();
+
+  static_assert(__is_same(decltype([]<class... Is>() {
+                            return ([]() -> decltype(Is()) { return {}; }(),
+                                    ...);
+                          }.template operator()<int, char>()),
+                          char));
+
+  []<class... Is>() {
+    return ([]<class... Ts>() -> decltype(Is()) { return Ts(); }() + ...);
+    // expected-error@-1 {{unexpanded parameter pack 'Ts'}}
+  }.template operator()<int, int>();
+
+  // https://github.com/llvm/llvm-project/issues/56852
+  []<class... Is>(Is...) {
+    ([] {
+      using T = identity<Is>::type;
+    }(), ...);
+  }(1, 2);
+
+  [](auto ...y) {
+    ([y] { }(), ...);
+  }();
+
+  [](auto ...x) {
+    ([&](auto ...y) {
+      ([x..., y] { }(), ...);
+    })(1);
+  }(2, 'b');
+
+#if 0
+  // FIXME: https://github.com/llvm/llvm-project/issues/18873
+  [](auto ...x) { // #1
+    ([&](auto ...y) {  // #2
+      ([x, y] { }(), ...); // #3
+    })(1, 'a');  // #4
+  }(2, 'b');  // #5
+
+  // We run into another crash for the above lambda because of the absence of a
+  // mechanism that rebuilds an unexpanded pack from an expanded Decls.
+  //
+  // Basically, this happens after `x` at #1 being expanded when the template
+  // arguments at #5, deduced as <int, char>, are ready. When we want to
+  // instantiate the body of #1, we first instantiate the CallExpr at #4, which
+  // boils down to the lambda's instantiation at #2. To that end, we have to
+  // instantiate the body of it, which turns out to be #3. #3 is a CXXFoldExpr,
+  // and we immediately have to hold off on the expansion because we don't have
+  // corresponding template arguments (arguments at #4 are not transformed yet) for it.
+  // Therefore, we want to rebuild a CXXFoldExpr, which requires another pattern
+  // transformation of the lambda inside #3. Then we need to find an unexpanded form
+  // of such a Decl of x at the time of transforming the capture, which is impossible
+  // because the instantiated form has been expanded at #1!
+
+  [](auto ...x) {  // #outer
+    ([&](auto ...y) { // #inner
+      ([x, y] { }(), ...);
+      // expected-error@-1 {{parameter pack 'y' that has a different length (4 vs. 3) from outer parameter packs}}
+      // expected-note-re@#inner {{function template specialization {{.*}} requested here}}
+      // expected-note-re@#outer {{function template specialization {{.*}} requested here}}
+      // expected-note-re@#instantiate-f {{function template specialization {{.*}} requested here}}
+    })('a', 'b', 'c');
+  }(0, 1, 2, 3);
+#endif
+}
+
+template void f(); // #instantiate-f
+
+} // namespace GH85667
+
+namespace GH99877 {
+
+struct tuple {
+  int x[3];
+};
+
+template <class F> int apply(F f, tuple v) { return f(v.x[0], v.x[1], v.x[2]); }
+
+int Cartesian1(auto x, auto y) {
+  return apply(
+      [&](auto... xs) {
+        return (apply([xs](auto... ys) { return (ys + ...); }, y) + ...);
+      },
+      x);
+}
+
+int Cartesian2(auto x, auto y) {
+  return apply(
+      [&](auto... xs) {
+        return (apply([zs = xs](auto... ys) { return (ys + ...); }, y) + ...);
+      },
+      x);
+}
+
+template <int...> struct Ints {};
+template <int> struct Choose {
+  template <class> struct Templ;
+};
+template <int... x> int Cartesian3(auto y) {
+  return [&]<int... xs>(Ints<xs...>) {
+    // check in default template arguments for
+    // - type template parameters,
+    (void)(apply([]<class = decltype(xs)>(auto... ys) { return (ys + ...); },
+                 y) +
+           ...);
+    // - template template parameters.
+    (void)(apply([]<template <class> class = Choose<xs>::template Templ>(
+                     auto... ys) { return (ys + ...); },
+                 y) +
+           ...);
+    // - non-type template parameters,
+    return (apply([]<int = xs>(auto... ys) { return (ys + ...); }, y) + ...);
+  }(Ints<x...>());
+}
+
+template <int... x> int Cartesian4(auto y) {
+  return [&]<int... xs>(Ints<xs...>) {
+    return (
+        apply([]<decltype(xs) xx = 1>(auto... ys) { return (ys + ...); }, y) +
+        ...);
+  }(Ints<x...>());
+}
+
+// FIXME: Attributes should preserve the ContainsUnexpandedPack flag.
+#if 0
+
+int Cartesian5(auto x, auto y) {
+  return apply(
+      [&](auto... xs) {
+        return (apply([](auto... ys) __attribute__((
+                          diagnose_if(!__is_same(decltype(xs), int), "message",
+                                      "error"))) { return (ys + ...); },
+                      y) +
+                ...);
+      },
+      x);
+}
+
+#endif
+
+void foo() {
+  auto x = tuple({1, 2, 3});
+  auto y = tuple({4, 5, 6});
+  Cartesian1(x, y);
+  Cartesian2(x, y);
+  Cartesian3<1, 2, 3>(y);
+  Cartesian4<1, 2, 3>(y);
+#if 0
+  Cartesian5(x, y);
+#endif
+}
+
+} // namespace GH99877