[clang] [Clang] [Sema] WIP: Fix dependence of DREs in lambdas with an explicit object parameter (PR #84473)

via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 8 04:55:48 PST 2024


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 at -1 {{cannot assign to a variable captured by copy in a non-mutable lambda}}
+    // expected-note at -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


More information about the cfe-commits mailing list