llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: None (Sirraide) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/84473.diff 3 Files Affected: - (modified) clang/lib/Sema/SemaExpr.cpp (+33-11) - (modified) clang/lib/Sema/TreeTransform.h (+2-2) - (modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+81) ``````````diff 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..e537dfef767df8 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->isCapturedByCopyInLambdaWithExplicitObjectParameter() && + 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 { `````````` </details> https://github.com/llvm/llvm-project/pull/84473 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
