[clang] 874067a - [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (#86265)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 5 19:54:48 PDT 2024
Author: Younan Zhang
Date: 2024-08-06T10:54:45+08:00
New Revision: 874067a22f0f483dbe210d8547c06d564bfa7848
URL: https://github.com/llvm/llvm-project/commit/874067a22f0f483dbe210d8547c06d564bfa7848
DIFF: https://github.com/llvm/llvm-project/commit/874067a22f0f483dbe210d8547c06d564bfa7848.diff
LOG: [Sema] Preserve ContainsUnexpandedParameterPack in TransformLambdaExpr (#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 https://github.com/llvm/llvm-project/issues/56852
Fixes https://github.com/llvm/llvm-project/issues/85667
Mitigates https://github.com/llvm/llvm-project/issues/99877 because the
attributes were not handled in this patch.
---------
Co-authored-by: Ilya Biryukov <809452+ilya-biryukov at users.noreply.github.com>
Co-authored-by: cor3ntin <corentinjabot at gmail.com>
Added:
clang/test/SemaCXX/fold_lambda_with_variadics.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0f1a4c1851911..54decb5dbf230 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 601077e9f3334..b697918120806 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/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);
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index de470739ab78e..9a6cd2cd0ab75 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/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) {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index bf38e59eae7fd..7f078d55eeb34 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/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>
diff --git a/clang/test/SemaCXX/fold_lambda_with_variadics.cpp b/clang/test/SemaCXX/fold_lambda_with_variadics.cpp
new file mode 100644
index 0000000000000..14e242f009dc5
--- /dev/null
+++ b/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 at -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 at -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 at -1 {{parameter pack 'y' that has a
diff erent 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
More information about the cfe-commits
mailing list