https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/84473
This is still WIP, but I figured I’d open a pr anyway to share what I’ve discovered so far. In short, dependence of captures in lambdas with an explicit object parameter is all kinds of broken at the moment. [[temp.dep.expr]](https://eel.is/c++draft/temp.dep.expr) states that > An > [id-expression](https://eel.is/c++draft/expr.prim.id.general#nt:id-expression) > is type-dependent if [...] its terminal name is > - associated by name lookup with an entity captured by copy > ([[expr.prim.lambda.capture]](https://eel.is/c++draft/expr.prim.lambda.capture)) > in a > [lambda-expression](https://eel.is/c++draft/expr.prim.lambda.general#nt:lambda-expression) > that has an explicit object parameter whose type is dependent > ([[dcl.fct]](https://eel.is/c++draft/dcl.fct)) In issue #84163, it was observed that, if the lambda object is `const` but the explicit object parameter is not `const` but is type-dependent, any by-value captures are not treated as `const`, even though they should be: ```c++ const auto l1 = [x](this auto&) { x = 42; }; l1(); // Should error because `x` ends up being `const`, but we don’t. ``` This seems to be due to several compounding issues: 1. we’re treating by-reference captures as dependent rather than by-value captures; 2. tree transform doesn’t seem to check whether referring to such a by-value capture makes a DRE dependent; 3. when checking whether a DRE refers to an such a by-value capture, we only check the immediately enclosing lambda, and not any parent lambdas; 4. we also don’t check for implicit by-value captures; 5. attempting to determine whether a lambda has an explicit object parameter by checking the `LambdaScopeInfo`’s `ExplicitObjectParameter` doesn’t seem to work properly; I presume this is because this member refers to a `ParmVarDecl` that isn’t populated (yet) when we perform template instantiaion, which causes issues with nested lambdas that each have an explicit object parameter. This pr should (eventually) fix all of these: 1. This is just due to a misplaced `!`, so that’s an easy fix. 2. This entails checking if the DRE is captured by value by a lambda with an explicit object parameter in `TreeTransform<Derived>::TransformDeclRefExpr`. That’s also an easy fix. 3. Currently, I’m simply iterating over all enclosing lambdas and checking if any of them capture the var decl that the DRE refers to by-value (explicitly or implicitly) rather than just looking at the parent lambda. 4. Check for implicit captures as well in 3. 5. Check for `isExplicitObjectMemberFunction()` on the lambda’s `CXXMethodDecl` instead. This seems to have fixed all issues with captured variables; however, capturing `this` by value (and then accessing NSDMs) suffers from the same problem. My current approach for a fix is adding a `CapturedByCopyInLambdaWithExplicitObjectParameter` to `CXXThisExpr` and making `this` type-dependent, which is how this is tracked for DREs, but that’s still WIP. If anyone has a better idea, feel free to let me know. Lastly, there’s also another issue (#84425) that seems to involve capturing lambdas with explicit object parameters; I have yet to determine if that one is related to this as well. This fixes #84163. >From 870e6a6def8c17859ffbb30906f91912268f872d Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Fri, 8 Mar 2024 11:55:42 +0100 Subject: [PATCH 1/2] [Clang] Fix dependence of DREs in lambdas with an explicit object parameter --- clang/lib/Sema/SemaExpr.cpp | 44 +++++++++--- clang/lib/Sema/TreeTransform.h | 4 +- clang/test/SemaCXX/cxx2b-deducing-this.cpp | 81 ++++++++++++++++++++++ 3 files changed, 116 insertions(+), 13 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 47bb263f56aade..700769860aa806 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -20687,20 +20687,42 @@ void Sema::MarkVariableReferenced(SourceLocation Loc, VarDecl *Var) { static void FixDependencyOfIdExpressionsInLambdaWithDependentObjectParameter( Sema &SemaRef, ValueDecl *D, Expr *E) { auto *ID = dyn_cast<DeclRefExpr>(E); - if (!ID || ID->isTypeDependent()) + if (!ID || ID->isTypeDependent() || !ID->refersToEnclosingVariableOrCapture()) return; + // If any enclosing lambda with a dependent explicit object parameter either + // explicitly captures the variable by value, or has a capture default of '=' + // and does not capture the variable by reference, then the type of the DRE + // is dependent on the type of that lambda's explicit object parameter. auto IsDependent = [&]() { - const LambdaScopeInfo *LSI = SemaRef.getCurLambda(); - if (!LSI) - return false; - if (!LSI->ExplicitObjectParameter || - !LSI->ExplicitObjectParameter->getType()->isDependentType()) - return false; - if (!LSI->CaptureMap.count(D)) - return false; - const Capture &Cap = LSI->getCapture(D); - return !Cap.isCopyCapture(); + for (auto *Scope : llvm::reverse(SemaRef.FunctionScopes)) { + auto *LSI = dyn_cast<sema::LambdaScopeInfo>(Scope); + if (!LSI) + continue; + + if (LSI->Lambda && !LSI->Lambda->Encloses(SemaRef.CurContext) && + LSI->AfterParameterList) + return false; + + const auto *MD = LSI->CallOperator; + if (MD->getType().isNull()) + continue; + + const auto *Ty = cast<FunctionProtoType>(MD->getType()); + if (!Ty || !MD->isExplicitObjectMemberFunction() || + !Ty->getParamType(0)->isDependentType()) + continue; + + if (auto *C = LSI->CaptureMap.count(D) ? &LSI->getCapture(D) : nullptr) { + if (C->isCopyCapture()) + return true; + continue; + } + + if (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByval) + return true; + } + return false; }(); ID->setCapturedByCopyInLambdaWithExplicitObjectParameter( diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 7389a48fe56fcc..867efd1ce5c80a 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -11099,8 +11099,8 @@ TreeTransform<Derived>::TransformDeclRefExpr(DeclRefExpr *E) { } if (!getDerived().AlwaysRebuild() && - QualifierLoc == E->getQualifierLoc() && - ND == E->getDecl() && + E->getDependence() == ExprDependence::None && + QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() && Found == E->getFoundDecl() && NameInfo.getName() == E->getDecl()->getDeclName() && !E->hasExplicitTemplateArgs()) { diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index b8ddb9ad300034..6c21954554d281 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -200,6 +200,87 @@ void TestMutationInLambda() { [i = 0](this auto){ i++; }(); [i = 0](this const auto&){ i++; }(); // expected-error@-1 {{cannot assign to a variable captured by copy in a non-mutable lambda}} + // expected-note@-2 {{in instantiation of}} + + int x; + const auto l1 = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + const auto l2 = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + + const auto l3 = [&x](this auto&) { + const auto l3a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l3a(); // expected-note {{in instantiation of}} + }; + + const auto l4 = [&x](this auto&) { + const auto l4a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l4a(); // expected-note {{in instantiation of}} + }; + + const auto l5 = [x](this auto&) { + const auto l5a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l5a(); // expected-note {{in instantiation of}} + }; + + const auto l6 = [=](this auto&) { + const auto l6a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l6a(); // expected-note {{in instantiation of}} + }; + + const auto l7 = [x](this auto&) { + const auto l7a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l7a(); // expected-note {{in instantiation of}} + }; + + const auto l8 = [=](this auto&) { + const auto l8a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l8a(); // expected-note {{in instantiation of}} + }; + + const auto l9 = [&](this auto&) { + const auto l9a = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l9a(); // expected-note {{in instantiation of}} + }; + + const auto l10 = [&](this auto&) { + const auto l10a = [=](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + l10a(); // expected-note {{in instantiation of}} + }; + + const auto l11 = [x](this auto&) { + const auto l11a = [&x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l11a(); + }; + + const auto l12 = [x](this auto&) { + const auto l12a = [&](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l12a(); + }; + + const auto l13 = [=](this auto&) { + const auto l13a = [&x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} expected-note {{while substituting}} + l13a(); + }; + + l1(); // expected-note {{in instantiation of}} + l2(); // expected-note {{in instantiation of}} + l3(); // expected-note {{in instantiation of}} + l4(); // expected-note {{in instantiation of}} + l5(); // expected-note {{in instantiation of}} + l6(); // expected-note {{in instantiation of}} + l7(); // expected-note {{in instantiation of}} + l8(); // expected-note {{in instantiation of}} + l9(); // expected-note {{in instantiation of}} + l10(); // expected-note {{in instantiation of}} + l11(); // expected-note {{in instantiation of}} + l12(); // expected-note {{in instantiation of}} + l13(); // expected-note {{in instantiation of}} + + { + const auto l1 = [&x](this auto&) { x = 42; }; + const auto l2 = [&](this auto&) { x = 42; }; + l1(); + l2(); + } } struct Over_Call_Func_Example { >From bf4c193f6e91d4dae4fa20c805b49955d95666cd Mon Sep 17 00:00:00 2001 From: Sirraide <aeternalm...@gmail.com> Date: Fri, 8 Mar 2024 13:37:57 +0100 Subject: [PATCH 2/2] [Clang] Better dependence check for DREs in TreeTransform --- clang/lib/Sema/TreeTransform.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 867efd1ce5c80a..e537dfef767df8 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -11099,7 +11099,7 @@ TreeTransform<Derived>::TransformDeclRefExpr(DeclRefExpr *E) { } if (!getDerived().AlwaysRebuild() && - E->getDependence() == ExprDependence::None && + !E->isCapturedByCopyInLambdaWithExplicitObjectParameter() && QualifierLoc == E->getQualifierLoc() && ND == E->getDecl() && Found == E->getFoundDecl() && NameInfo.getName() == E->getDecl()->getDeclName() && _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
