[clang] [Clang][Sema] Tweak tryCaptureVariable for unevaluated lambdas (PR #93206)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 3 02:53:45 PDT 2024
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/93206
>From 658e9d46adf6dd79aa6aef03a1817444a880348a Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 23 May 2024 22:35:11 +0800
Subject: [PATCH 1/3] [Clang][Sema] Tweak tryCaptureVariable for unevaluated
lambdas
This patch picks up #78598 with the hope that we can address such
crashes in tryCaptureVariable for unevaluated lambdas.
In addition to tryCaptureVariable, this also contains several other
fixes on e.g. lambda parsing/dependencies.
---
clang/docs/ReleaseNotes.rst | 3 +
clang/include/clang/AST/DeclBase.h | 4 +
clang/lib/Parse/ParseExprCXX.cpp | 5 +-
clang/lib/Sema/SemaExpr.cpp | 10 ++
clang/lib/Sema/SemaLambda.cpp | 20 +++
clang/lib/Sema/TreeTransform.h | 2 +-
clang/test/SemaCXX/lambda-unevaluated.cpp | 164 ++++++++++++++++++++++
7 files changed, 206 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 49ab222bec405..e06a5cd9536aa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -710,6 +710,9 @@ Bug Fixes to C++ Support
- Correctly treat the compound statement of an ``if consteval`` as an immediate context. Fixes (#GH91509).
- When partial ordering alias templates against template template parameters,
allow pack expansions when the alias has a fixed-size parameter list. Fixes (#GH62529).
+- Fixes several bugs in capturing variables within unevaluated contexts. Fixes (#GH63845), (#GH67260), (#GH69307), (#GH88081),
+ (#GH89496), (#GH90669) and (#GH91633).
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index e43e812cd9455..3a311d4c55916 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -2148,6 +2148,10 @@ class DeclContext {
getDeclKind() <= Decl::lastRecord;
}
+ bool isRequiresExprBody() const {
+ return getDeclKind() == Decl::RequiresExprBody;
+ }
+
bool isNamespace() const { return getDeclKind() == Decl::Namespace; }
bool isStdNamespace() const;
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 825031da358ad..73f619a085b04 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1576,7 +1576,10 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
TrailingReturnTypeLoc, &DS),
std::move(Attributes), DeclEndLoc);
- Actions.ActOnLambdaClosureQualifiers(Intro, MutableLoc);
+ // We have called ActOnLambdaClosureQualifiers for parentheses-less cases
+ // above.
+ if (HasParentheses)
+ Actions.ActOnLambdaClosureQualifiers(Intro, MutableLoc);
if (HasParentheses && Tok.is(tok::kw_requires))
ParseTrailingRequiresClause(D);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e6c3fa51d54da..7a4ede9898034 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -18831,6 +18831,10 @@ bool Sema::tryCaptureVariable(
DeclContext *VarDC = Var->getDeclContext();
DeclContext *DC = CurContext;
+ // Skip past RequiresExprBodys because they don't constitute function scopes.
+ while (DC->isRequiresExprBody())
+ DC = DC->getParent();
+
// tryCaptureVariable is called every time a DeclRef is formed,
// it can therefore have non-negigible impact on performances.
// For local variables and when there is no capturing scope,
@@ -18838,6 +18842,12 @@ bool Sema::tryCaptureVariable(
if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC))
return true;
+ // Exception: Function parameters are not tied to the function's DeclContext
+ // until we enter the function definition. Capturing them anyway would result
+ // in an out-of-bounds error while traversing DC and its parents.
+ if (isa<ParmVarDecl>(Var) && !VarDC->isFunctionOrMethod())
+ return true;
+
const auto *VD = dyn_cast<VarDecl>(Var);
if (VD) {
if (VD->isInitCapture())
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 1743afaf15287..999316e544b91 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1036,6 +1036,7 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
// be dependent, because there are template parameters in scope.
CXXRecordDecl::LambdaDependencyKind LambdaDependencyKind =
CXXRecordDecl::LDK_Unknown;
+
if (LSI->NumExplicitTemplateParams > 0) {
Scope *TemplateParamScope = CurScope->getTemplateParamParent();
assert(TemplateParamScope &&
@@ -1046,6 +1047,25 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
} else if (CurScope->getTemplateParamParent() != nullptr) {
LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
+ } else if (Scope *P = CurScope->getParent()) {
+ // Given a lambda defined inside a requires expression,
+ //
+ // struct S {
+ // S(auto var) requires requires { [&] -> decltype(var) { }; }
+ // {}
+ // };
+ //
+ // The parameter var is not injected into the function Decl at the point of
+ // parsing lambda. In such scenarios, perceiving it as dependent could
+ // result in the constraint being evaluated, which matches what GCC does.
+ while (P->getEntity() && P->getEntity()->isRequiresExprBody())
+ P = P->getParent();
+ if (P->isFunctionDeclarationScope() &&
+ llvm::any_of(P->decls(), [](Decl *D) {
+ return isa<ParmVarDecl>(D) &&
+ cast<ParmVarDecl>(D)->getType()->isTemplateTypeParmType();
+ }))
+ LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
}
CXXRecordDecl *Class = createLambdaClosureType(
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index c039b95293af2..bff4377e34110 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14232,7 +14232,7 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
// will be deemed as dependent even if there are no dependent template
// arguments.
// (A ClassTemplateSpecializationDecl is always a dependent context.)
- while (DC->getDeclKind() == Decl::Kind::RequiresExprBody)
+ while (DC->isRequiresExprBody())
DC = DC->getParent();
if ((getSema().isUnevaluatedContext() ||
getSema().isConstantEvaluatedContext()) &&
diff --git a/clang/test/SemaCXX/lambda-unevaluated.cpp b/clang/test/SemaCXX/lambda-unevaluated.cpp
index 10d4c2228ec9b..0d21f4b13bca3 100644
--- a/clang/test/SemaCXX/lambda-unevaluated.cpp
+++ b/clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -189,3 +189,167 @@ void recursive() {
}
}
+
+namespace GH63845 {
+
+template <bool> struct A {};
+
+struct true_type {
+ constexpr operator bool() noexcept { return true; }
+};
+
+constexpr bool foo() {
+ true_type x{};
+ return requires { typename A<x>; }; // fails in Clang
+}
+
+constexpr bool foo(auto b) {
+ return requires { typename A<b>; };
+}
+
+static_assert(foo(true_type{}));
+static_assert(foo());
+
+}
+
+namespace GH88081 {
+
+// Test that ActOnLambdaClosureQualifiers is called only once.
+void foo(auto value)
+ requires requires { [&] -> decltype(value) {}; }
+ // expected-error at -1 {{non-local lambda expression cannot have a capture-default}}
+{}
+
+struct S { //#S
+ S(auto value) //#S-ctor
+ requires requires { [&] -> decltype(value) { return 2; }; } {} // #S-requires
+
+ static auto foo(auto value) -> decltype([&]() -> decltype(value) {}()) { return {}; } // #S-foo
+
+ static auto bar(auto value) -> decltype([&] { return value; }()) {
+ return "a"; // #bar-body
+ }
+};
+
+S s("a"); // #use
+// expected-error@#S-requires {{cannot initialize return object of type 'decltype(value)' (aka 'const char *') with an rvalue of type 'int'}}
+// expected-error@#use {{no matching constructor}}
+// expected-note@#S-requires {{substituting into a lambda expression here}}
+// expected-note@#S-requires {{substituting template arguments into constraint expression here}}
+// expected-note@#S-requires {{in instantiation of requirement here}}
+// expected-note@#use {{checking constraint satisfaction for template 'S<const char *>' required here}}
+// expected-note@#use {{requested here}}
+// expected-note-re@#S 2{{candidate constructor {{.*}} not viable}}
+// expected-note@#S-ctor {{constraints not satisfied}}
+// expected-note-re@#S-requires {{because {{.*}} would be invalid}}
+
+void func() {
+ S::foo(42);
+ S::bar("str");
+ S::bar(0.618);
+ // expected-error-re@#bar-body {{cannot initialize return object of type {{.*}} (aka 'double') with an lvalue of type 'const char[2]'}}
+ // expected-note at -2 {{requested here}}
+}
+
+} // namespace GH88081
+
+namespace GH69307 {
+
+constexpr auto ICE() {
+ constexpr auto b = 1;
+ return [=](auto c) -> int
+ requires requires { b + c; }
+ { return 1; };
+};
+
+constexpr auto Ret = ICE()(1);
+
+constexpr auto ICE(auto a) { // template function, not lambda
+ return [=]()
+ requires requires { a; }
+ { return 1; };
+};
+
+} // namespace GH69307
+
+namespace GH91633 {
+
+struct __normal_iterator {};
+
+template <typename _Iterator>
+void operator==(_Iterator __lhs, _Iterator) // expected-note {{declared here}}
+ requires requires { __lhs; };
+
+__normal_iterator finder();
+
+template <typename >
+void findDetail() {
+ auto makeResult = [&](auto foo) -> void {
+ finder() != foo;
+ // expected-error at -1 {{function for rewritten '!=' comparison is not 'bool'}}
+ };
+ makeResult(__normal_iterator{}); // expected-note {{requested here}}
+}
+
+void find() {
+ findDetail<void>(); // expected-note {{requested here}}
+}
+} // namespace GH91633
+
+namespace GH90669 {
+
+struct __normal_iterator {};
+
+struct vector {
+ __normal_iterator begin(); // #begin
+ int end();
+};
+
+template <typename _IteratorR>
+bool operator==(_IteratorR __lhs, int)
+ requires requires { __lhs; }
+{}
+
+template <typename PrepareFunc> void queued_for_each(PrepareFunc prep) {
+ prep(vector{}); //#prep
+}
+
+void scan() {
+ queued_for_each([&](auto ino) -> int { // #queued-for-each
+ for (auto e : ino) { // #for-each
+ // expected-error@#for-each {{cannot increment value of type '__normal_iterator'}}
+ // expected-note-re@#prep {{instantiation {{.*}} requested here}}
+ // expected-note-re@#queued-for-each {{instantiation {{.*}} requested here}}
+ // expected-note@#for-each {{implicit call to 'operator++'}}
+ // expected-note@#begin {{selected 'begin' function}}
+ };
+ });
+}
+} // namespace GH90669
+
+namespace GH89496 {
+template <class Iter> struct RevIter {
+ Iter current;
+ constexpr explicit RevIter(Iter x) : current(x) {}
+ inline constexpr bool operator==(const RevIter<Iter> &other) const
+ requires requires {
+ // { current == other.current } -> std::convertible_to<bool>;
+ { other };
+ }
+ {
+ return true;
+ }
+};
+struct Foo {
+ Foo() {};
+};
+void CrashFunc() {
+ auto lambda = [&](auto from, auto to) -> Foo {
+ (void)(from == to);
+ return Foo();
+ };
+ auto from = RevIter<int *>(nullptr);
+ auto to = RevIter<int *>(nullptr);
+ lambda(from, to);
+}
+} // namespace pr89496
>From 60aff76c5aa332ef383777e2c45b7a093eefe817 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 3 Jun 2024 17:38:59 +0800
Subject: [PATCH 2/3] Cleanup duplicate tests
---
clang/test/SemaCXX/lambda-unevaluated.cpp | 129 ++++------------------
1 file changed, 20 insertions(+), 109 deletions(-)
diff --git a/clang/test/SemaCXX/lambda-unevaluated.cpp b/clang/test/SemaCXX/lambda-unevaluated.cpp
index 0d21f4b13bca3..39ee89bc797f8 100644
--- a/clang/test/SemaCXX/lambda-unevaluated.cpp
+++ b/clang/test/SemaCXX/lambda-unevaluated.cpp
@@ -190,6 +190,7 @@ void recursive() {
}
}
+// GH63845: Test if we have skipped past RequiresExprBodyDecls in tryCaptureVariable().
namespace GH63845 {
template <bool> struct A {};
@@ -200,21 +201,31 @@ struct true_type {
constexpr bool foo() {
true_type x{};
- return requires { typename A<x>; }; // fails in Clang
+ return requires { typename A<x>; };
}
-constexpr bool foo(auto b) {
- return requires { typename A<b>; };
-}
-
-static_assert(foo(true_type{}));
static_assert(foo());
-}
+} // namespace GH63845
+
+// GH69307: Test if we can correctly handle param decls that have yet to get into the function scope.
+namespace GH69307 {
+
+constexpr auto ICE() {
+ constexpr auto b = 1;
+ return [=](auto c) -> int
+ requires requires { b + c; }
+ { return 1; };
+};
+
+constexpr auto Ret = ICE()(1);
+} // namespace GH69307
+
+// GH88081: Test if we evaluate the requires expression with lambda captures properly.
namespace GH88081 {
-// Test that ActOnLambdaClosureQualifiers is called only once.
+// Test that ActOnLambdaClosureQualifiers() is called only once.
void foo(auto value)
requires requires { [&] -> decltype(value) {}; }
// expected-error at -1 {{non-local lambda expression cannot have a capture-default}}
@@ -226,6 +237,7 @@ struct S { //#S
static auto foo(auto value) -> decltype([&]() -> decltype(value) {}()) { return {}; } // #S-foo
+ // FIXME: 'value' does not constitute an ODR use here. Add a diagnostic for it.
static auto bar(auto value) -> decltype([&] { return value; }()) {
return "a"; // #bar-body
}
@@ -252,104 +264,3 @@ void func() {
}
} // namespace GH88081
-
-namespace GH69307 {
-
-constexpr auto ICE() {
- constexpr auto b = 1;
- return [=](auto c) -> int
- requires requires { b + c; }
- { return 1; };
-};
-
-constexpr auto Ret = ICE()(1);
-
-constexpr auto ICE(auto a) { // template function, not lambda
- return [=]()
- requires requires { a; }
- { return 1; };
-};
-
-} // namespace GH69307
-
-namespace GH91633 {
-
-struct __normal_iterator {};
-
-template <typename _Iterator>
-void operator==(_Iterator __lhs, _Iterator) // expected-note {{declared here}}
- requires requires { __lhs; };
-
-__normal_iterator finder();
-
-template <typename >
-void findDetail() {
- auto makeResult = [&](auto foo) -> void {
- finder() != foo;
- // expected-error at -1 {{function for rewritten '!=' comparison is not 'bool'}}
- };
- makeResult(__normal_iterator{}); // expected-note {{requested here}}
-}
-
-void find() {
- findDetail<void>(); // expected-note {{requested here}}
-}
-} // namespace GH91633
-
-namespace GH90669 {
-
-struct __normal_iterator {};
-
-struct vector {
- __normal_iterator begin(); // #begin
- int end();
-};
-
-template <typename _IteratorR>
-bool operator==(_IteratorR __lhs, int)
- requires requires { __lhs; }
-{}
-
-template <typename PrepareFunc> void queued_for_each(PrepareFunc prep) {
- prep(vector{}); //#prep
-}
-
-void scan() {
- queued_for_each([&](auto ino) -> int { // #queued-for-each
- for (auto e : ino) { // #for-each
- // expected-error@#for-each {{cannot increment value of type '__normal_iterator'}}
- // expected-note-re@#prep {{instantiation {{.*}} requested here}}
- // expected-note-re@#queued-for-each {{instantiation {{.*}} requested here}}
- // expected-note@#for-each {{implicit call to 'operator++'}}
- // expected-note@#begin {{selected 'begin' function}}
- };
- });
-}
-} // namespace GH90669
-
-namespace GH89496 {
-template <class Iter> struct RevIter {
- Iter current;
- constexpr explicit RevIter(Iter x) : current(x) {}
- inline constexpr bool operator==(const RevIter<Iter> &other) const
- requires requires {
- // { current == other.current } -> std::convertible_to<bool>;
- { other };
- }
- {
- return true;
- }
-};
-struct Foo {
- Foo() {};
-};
-void CrashFunc() {
- auto lambda = [&](auto from, auto to) -> Foo {
- (void)(from == to);
- return Foo();
- };
- auto from = RevIter<int *>(nullptr);
- auto to = RevIter<int *>(nullptr);
- lambda(from, to);
-}
-} // namespace pr89496
>From 36d6277f702affc1e449a30fc81b405913467b2d Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 3 Jun 2024 17:53:07 +0800
Subject: [PATCH 3/3] Remove dead codes in ActOnLambdaExpressionAfterIntroducer
---
clang/lib/Sema/SemaLambda.cpp | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp
index 999316e544b91..157d9fa1c97bc 100644
--- a/clang/lib/Sema/SemaLambda.cpp
+++ b/clang/lib/Sema/SemaLambda.cpp
@@ -1036,16 +1036,7 @@ void Sema::ActOnLambdaExpressionAfterIntroducer(LambdaIntroducer &Intro,
// be dependent, because there are template parameters in scope.
CXXRecordDecl::LambdaDependencyKind LambdaDependencyKind =
CXXRecordDecl::LDK_Unknown;
-
- if (LSI->NumExplicitTemplateParams > 0) {
- Scope *TemplateParamScope = CurScope->getTemplateParamParent();
- assert(TemplateParamScope &&
- "Lambda with explicit template param list should establish a "
- "template param scope");
- assert(TemplateParamScope->getParent());
- if (TemplateParamScope->getParent()->getTemplateParamParent() != nullptr)
- LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
- } else if (CurScope->getTemplateParamParent() != nullptr) {
+ if (CurScope->getTemplateParamParent() != nullptr) {
LambdaDependencyKind = CXXRecordDecl::LDK_AlwaysDependent;
} else if (Scope *P = CurScope->getParent()) {
// Given a lambda defined inside a requires expression,
More information about the cfe-commits
mailing list