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 <zyn7...@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@-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@-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@-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 <zyn7...@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@-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@-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 <zyn7...@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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits