[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
shafik wrote: > @cor3ntin @shafik Hi, I want to take charge of this issue and submit a PR for > the fix. I would open a new PR and reference this one in the summary for completeness. It looks like this one is not going to picked up by the author and so if you can take it over and finish it that would be very much appreciated. There are a lot of issues that crash similarly and so we likely have plenty of tests to try against a fix :-) https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
l1nxy wrote: @cor3ntin @shafikHi, I want to take charge of this issue and submit a PR for the fix. https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
cor3ntin wrote: If @HerrCai0907 doesn't reply, we should take over @shafik @erichkeane @Endilll https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
zyn0217 wrote: Could you please explain why you're closing the PR? https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 closed https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
LYP951018 wrote: This PR should also be able to fix https://github.com/llvm/llvm-project/issues/63972. https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
@@ -19706,22 +19706,29 @@ static void buildLambdaCaptureFixit(Sema &Sema, LambdaScopeInfo *LSI, } } +static DeclContext *ignoreReuquiresBodyDecl(DeclContext *DC) { erichkeane wrote: Name here doesn't match what you're doing here. Perhaps something closer to `getParentIgnoringRequiresBodyDecl`? Not a great name either, but better I think? https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
HerrCai0907 wrote: ping @erichkeane @cor3ntin https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
HerrCai0907 wrote: > Thanks for the patch! > > Requires Expressions should be in an unevaluated context, so we should never > try to capture variables they mentioned. > > Maybe we need to test the evaluation context in either `tryCaptureVariable` > or `NeedToCaptureVariable`, rather than a specific handling for requires > expressions > > https://eel.is/c++draft/expr.prim.lambda.capture#7.sentence-2 > > WDYT? > > @erichkeane We will use `tryCaptureVariable` to diagnose `-Wunused-lambda-capture`. So simply ignoring them in an unevaluated context is not a good idea. I have changed the PR to match this function work even for `RequiresExprBodyDecl`. https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/78598 >From 8fa3dc43e770025308da47f6aff309fa58c47fc3 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Thu, 18 Jan 2024 23:12:23 +0800 Subject: [PATCH 1/3] [clang] reject to capture variable in `RequiresExprBodyDecl` Expression in `RequiresExprBodyDecl` is resolved as constants and should not be captured. Fixes: #69307, #76593. --- clang/lib/Sema/SemaExpr.cpp | 21 ++-- ...uires-expression-with-capture-variable.cpp | 25 +++ 2 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 clang/test/SemaCXX/requires-expression-with-capture-variable.cpp diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6413a48f809ac9c..580e759f634956b 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19704,6 +19704,12 @@ static void buildLambdaCaptureFixit(Sema &Sema, LambdaScopeInfo *LSI, } } +static DeclContext *ignoreReuquiresBodyDecl(DeclContext *DC) { + if (isa_and_present(DC)) +return DC->getParent(); + return DC; +} + bool Sema::tryCaptureVariable( ValueDecl *Var, SourceLocation ExprLoc, TryCaptureKind Kind, SourceLocation EllipsisLoc, bool BuildAndDiagnose, QualType &CaptureType, @@ -19711,15 +19717,15 @@ bool Sema::tryCaptureVariable( // An init-capture is notionally from the context surrounding its // declaration, but its parent DC is the lambda class. DeclContext *VarDC = Var->getDeclContext(); - DeclContext *DC = CurContext; - // 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, // we can bailout early. - if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC)) + if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == CurContext)) return true; + DeclContext *DC = ignoreReuquiresBodyDecl(CurContext); + const auto *VD = dyn_cast(Var); if (VD) { if (VD->isInitCapture()) @@ -19789,11 +19795,10 @@ bool Sema::tryCaptureVariable( // Only block literals, captured statements, and lambda expressions can // capture; other scopes don't work. -DeclContext *ParentDC = -!IsInScopeDeclarationContext -? DC->getParent() -: getParentOfCapturingContextOrNull(DC, Var, ExprLoc, -BuildAndDiagnose, *this); +DeclContext *ParentDC = IsInScopeDeclarationContext +? getParentOfCapturingContextOrNull( + DC, Var, ExprLoc, BuildAndDiagnose, *this) +: DC->getParent(); // We need to check for the parent *first* because, if we *have* // private-captured a global variable, we need to recursively capture it in // intermediate blocks, lambdas, etc. diff --git a/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp new file mode 100644 index 000..d01a54133f6c39b --- /dev/null +++ b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp @@ -0,0 +1,25 @@ +// RUN: %clang -fsyntax-only -std=c++20 -Xclang -verify %s + +// expected-no-diagnostics + +auto GH69307_Func_1() { + constexpr auto b = 1; + return [&](auto c) -> int + requires requires { b + c; } + { return 1; }; +}; +auto GH69307_Func_Ret = GH69307_Func_1()(1); + +auto GH69307_Lambda_1 = []() { + return [&](auto c) -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_1_Ret = GH69307_Lambda_1()(1); + +auto GH69307_Lambda_2 = [](auto c) { + return [&]() -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_2_Ret = GH69307_Lambda_2(1)(); >From a99f16ff51702ff249cdf8a47de0cf24a08f694a Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sat, 20 Jan 2024 13:40:12 +0800 Subject: [PATCH 2/3] another test --- clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp | 7 +++ clang/test/SemaCXX/warn-unused-lambda-capture.cpp | 4 2 files changed, 11 insertions(+) create mode 100644 clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp diff --git a/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp b/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp new file mode 100644 index 000..b787edb188ed8b5 --- /dev/null +++ b/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -Wunused-lambda-capture -Wused-but-marked-unused -Wno-uninitialized -verify -std=c++20 %s + +void test() { + int i; + auto explicit_by_value_unused_requires = [i] (auto) requires requires { i; } {}; // expected-warning{{lambda capture 'i' is not required to be captured for this use}} + explicit_by_value_unused_requires(1); +} diff --
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 edited https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 edited https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff e611a4cf8060bf0a95b4acd9e136733425da085a a99f16ff51702ff249cdf8a47de0cf24a08f694a -- clang/test/SemaCXX/requires-expression-with-capture-variable.cpp clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/warn-unused-lambda-capture.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 580e759f63..04cfe5435e 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19721,7 +19721,8 @@ bool Sema::tryCaptureVariable( // it can therefore have non-negigible impact on performances. // For local variables and when there is no capturing scope, // we can bailout early. - if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == CurContext)) + if (CapturingFunctionScopes == 0 && + (!BuildAndDiagnose || VarDC == CurContext)) return true; DeclContext *DC = ignoreReuquiresBodyDecl(CurContext); `` https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/78598 >From 8fa3dc43e770025308da47f6aff309fa58c47fc3 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Thu, 18 Jan 2024 23:12:23 +0800 Subject: [PATCH 1/2] [clang] reject to capture variable in `RequiresExprBodyDecl` Expression in `RequiresExprBodyDecl` is resolved as constants and should not be captured. Fixes: #69307, #76593. --- clang/lib/Sema/SemaExpr.cpp | 21 ++-- ...uires-expression-with-capture-variable.cpp | 25 +++ 2 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 clang/test/SemaCXX/requires-expression-with-capture-variable.cpp diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6413a48f809ac9..580e759f634956 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19704,6 +19704,12 @@ static void buildLambdaCaptureFixit(Sema &Sema, LambdaScopeInfo *LSI, } } +static DeclContext *ignoreReuquiresBodyDecl(DeclContext *DC) { + if (isa_and_present(DC)) +return DC->getParent(); + return DC; +} + bool Sema::tryCaptureVariable( ValueDecl *Var, SourceLocation ExprLoc, TryCaptureKind Kind, SourceLocation EllipsisLoc, bool BuildAndDiagnose, QualType &CaptureType, @@ -19711,15 +19717,15 @@ bool Sema::tryCaptureVariable( // An init-capture is notionally from the context surrounding its // declaration, but its parent DC is the lambda class. DeclContext *VarDC = Var->getDeclContext(); - DeclContext *DC = CurContext; - // 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, // we can bailout early. - if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC)) + if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == CurContext)) return true; + DeclContext *DC = ignoreReuquiresBodyDecl(CurContext); + const auto *VD = dyn_cast(Var); if (VD) { if (VD->isInitCapture()) @@ -19789,11 +19795,10 @@ bool Sema::tryCaptureVariable( // Only block literals, captured statements, and lambda expressions can // capture; other scopes don't work. -DeclContext *ParentDC = -!IsInScopeDeclarationContext -? DC->getParent() -: getParentOfCapturingContextOrNull(DC, Var, ExprLoc, -BuildAndDiagnose, *this); +DeclContext *ParentDC = IsInScopeDeclarationContext +? getParentOfCapturingContextOrNull( + DC, Var, ExprLoc, BuildAndDiagnose, *this) +: DC->getParent(); // We need to check for the parent *first* because, if we *have* // private-captured a global variable, we need to recursively capture it in // intermediate blocks, lambdas, etc. diff --git a/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp new file mode 100644 index 00..d01a54133f6c39 --- /dev/null +++ b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp @@ -0,0 +1,25 @@ +// RUN: %clang -fsyntax-only -std=c++20 -Xclang -verify %s + +// expected-no-diagnostics + +auto GH69307_Func_1() { + constexpr auto b = 1; + return [&](auto c) -> int + requires requires { b + c; } + { return 1; }; +}; +auto GH69307_Func_Ret = GH69307_Func_1()(1); + +auto GH69307_Lambda_1 = []() { + return [&](auto c) -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_1_Ret = GH69307_Lambda_1()(1); + +auto GH69307_Lambda_2 = [](auto c) { + return [&]() -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_2_Ret = GH69307_Lambda_2(1)(); >From a99f16ff51702ff249cdf8a47de0cf24a08f694a Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Sat, 20 Jan 2024 13:40:12 +0800 Subject: [PATCH 2/2] another test --- clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp | 7 +++ clang/test/SemaCXX/warn-unused-lambda-capture.cpp | 4 2 files changed, 11 insertions(+) create mode 100644 clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp diff --git a/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp b/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp new file mode 100644 index 00..b787edb188ed8b --- /dev/null +++ b/clang/test/SemaCXX/warn-unused-lambda-capture-cxx20.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -fsyntax-only -Wunused-lambda-capture -Wused-but-marked-unused -Wno-uninitialized -verify -std=c++20 %s + +void test() { + int i; + auto explicit_by_value_unused_requires = [i] (auto) requires requires { i; } {}; // expected-warning{{lambda capture 'i' is not required to be captured for this use}} + explicit_by_value_unused_requires(1); +} diff --git a/
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
zyn0217 wrote: I think this also fixes https://github.com/llvm/llvm-project/issues/63845 and the example in https://github.com/llvm/llvm-project/issues/41751#issuecomment-1696389046. https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
cor3ntin wrote: Thanks for the patch! Requires Expressions should be in an unevaluated context, so we should never try to capture variables they mentioned. Maybe we need to test the evaluation context in either `tryCaptureVariable` or `NeedToCaptureVariable`, rather than a specific handling for requires expressions https://eel.is/c++draft/expr.prim.lambda.capture#7.sentence-2 WDYT? @erichkeane https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Congcong Cai (HerrCai0907) Changes Expression in `RequiresExprBodyDecl` is resolved as constants and should not be captured. Fixes: #69307, #76593. --- Full diff: https://github.com/llvm/llvm-project/pull/78598.diff 2 Files Affected: - (modified) clang/lib/Sema/SemaExpr.cpp (+3-1) - (added) clang/test/SemaCXX/requires-expression-with-capture-variable.cpp (+25) ``diff diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6413a48f809ac9..767fc3787cb22f 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19719,7 +19719,9 @@ bool Sema::tryCaptureVariable( // we can bailout early. if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC)) return true; - + // Expression in `RequiresExprBodyDecl` should not be captured. + if (isa(CurContext)) +return true; const auto *VD = dyn_cast(Var); if (VD) { if (VD->isInitCapture()) diff --git a/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp new file mode 100644 index 00..d01a54133f6c39 --- /dev/null +++ b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp @@ -0,0 +1,25 @@ +// RUN: %clang -fsyntax-only -std=c++20 -Xclang -verify %s + +// expected-no-diagnostics + +auto GH69307_Func_1() { + constexpr auto b = 1; + return [&](auto c) -> int + requires requires { b + c; } + { return 1; }; +}; +auto GH69307_Func_Ret = GH69307_Func_1()(1); + +auto GH69307_Lambda_1 = []() { + return [&](auto c) -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_1_Ret = GH69307_Lambda_1()(1); + +auto GH69307_Lambda_2 = [](auto c) { + return [&]() -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_2_Ret = GH69307_Lambda_2(1)(); `` https://github.com/llvm/llvm-project/pull/78598 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] reject to capture variable in `RequiresExprBodyDecl` (PR #78598)
https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/78598 Expression in `RequiresExprBodyDecl` is resolved as constants and should not be captured. Fixes: #69307, #76593. >From 33db497c31fd9da3a3acfc0d419dcdc396752863 Mon Sep 17 00:00:00 2001 From: Congcong Cai Date: Thu, 18 Jan 2024 23:12:23 +0800 Subject: [PATCH] [clang] reject to capture variable in `RequiresExprBodyDecl` Expression in `RequiresExprBodyDecl` is resolved as constants and should not be captured. Fixes: #69307, #76593. --- clang/lib/Sema/SemaExpr.cpp | 4 ++- ...uires-expression-with-capture-variable.cpp | 25 +++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 clang/test/SemaCXX/requires-expression-with-capture-variable.cpp diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6413a48f809ac9..767fc3787cb22f 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19719,7 +19719,9 @@ bool Sema::tryCaptureVariable( // we can bailout early. if (CapturingFunctionScopes == 0 && (!BuildAndDiagnose || VarDC == DC)) return true; - + // Expression in `RequiresExprBodyDecl` should not be captured. + if (isa(CurContext)) +return true; const auto *VD = dyn_cast(Var); if (VD) { if (VD->isInitCapture()) diff --git a/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp new file mode 100644 index 00..d01a54133f6c39 --- /dev/null +++ b/clang/test/SemaCXX/requires-expression-with-capture-variable.cpp @@ -0,0 +1,25 @@ +// RUN: %clang -fsyntax-only -std=c++20 -Xclang -verify %s + +// expected-no-diagnostics + +auto GH69307_Func_1() { + constexpr auto b = 1; + return [&](auto c) -> int + requires requires { b + c; } + { return 1; }; +}; +auto GH69307_Func_Ret = GH69307_Func_1()(1); + +auto GH69307_Lambda_1 = []() { + return [&](auto c) -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_1_Ret = GH69307_Lambda_1()(1); + +auto GH69307_Lambda_2 = [](auto c) { + return [&]() -> int + requires requires { c; } + { return 1; }; +}; +auto GH69307_Lambda_2_Ret = GH69307_Lambda_2(1)(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits