Skip to content

Permitted assignment to read-only capture in not-mutable lambda #95081

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Fedr opened this issue Jun 11, 2024 · 4 comments · Fixed by #120849
Closed

Permitted assignment to read-only capture in not-mutable lambda #95081

Fedr opened this issue Jun 11, 2024 · 4 comments · Fixed by #120849
Labels
accepts-invalid clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party lambda C++11 lambda expressions

Comments

@Fedr
Copy link

Fedr commented Jun 11, 2024

This program

struct B {
    int i;
};

int foo() {
    auto [x] = B{1};
    [x]() {
        x = 2;
    }();
    return x;
}

is invalid because not-mutable lambda modifies its read-only capture x, and it is properly rejected by GCC and MSVC, but Clang erroneously admits it. Online demo: https://gcc.godbolt.org/z/46o6jad8n

@Fedr Fedr changed the title Permitted assignment of read-only capture in not-mutable lambda Permitted assignment to read-only capture in not-mutable lambda Jun 11, 2024
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" accepts-invalid lambda C++11 lambda expressions and removed new issue labels Jun 11, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 11, 2024

@llvm/issue-subscribers-clang-frontend

Author: Fedor Chelnokov (Fedr)

This program ``` struct B { int i; };

int foo() {
auto [x] = B{1};
x {
x = 2;
}();
return x;
}

is invalid because not-mutable lambda modifies its read-only capture `x`, and it is properly rejected by GCC and MSVC, but Clang erroneously admits it. Online demo: https://gcc.godbolt.org/z/46o6jad8n
</details>

@shafik
Copy link
Collaborator

shafik commented Jun 11, 2024

If we use x=x instead, we obtain the diagnostic we expect: https://gcc.godbolt.org/z/MbjY4a1j8

Which is err_lambda_decl_ref_not_modifiable_lvalue emitted from here:

DiagID = diag::err_lambda_decl_ref_not_modifiable_lvalue;

It looks like when running E->isModifiableLvalue(...) we end up eventually here:

CanQualType CT = Ctx.getCanonicalType(E->getType());
// Const stuff is obviously not modifiable.
if (CT.isConstQualified())
return Cl::CM_ConstQualified;

In the OP's case CT.isConstQualified() is false and I don't see any other point where it could/should catch this case.

@shafik shafik added the confirmed Verified by a second party label Jun 11, 2024
@zygoloid
Copy link
Collaborator

We're computing the wrong type for the DeclRefExpr. Eg, we reject-valid on this:

void f(int&) = delete;
void f(const int&);

int arr[1];
void foo() {
    auto [x] = arr;
    [x]() { f(x); }();
}

@shafik
Copy link
Collaborator

shafik commented Jun 11, 2024

At this point:

switch (D->getKind()) {

for the x=x the result of D->getKind() is Var but is Binding for the OP's case.

So it looks like we skip the getCapturedDeclRefType(cast<VarDecl>(VD), Loc); in the OP's case.

When we do the lookup here:

LookupResult Result(*this, Found->getDeclName(), NameLoc, LookupOrdinaryName);
Result.addDecl(Found);
Result.resolveKind();

We are missing the capture in the OP's case.

github-actions bot pushed a commit to arm/arm-toolchain that referenced this issue Jan 10, 2025
… inside immutable lambda (#120849)

For structured bindings, a call to getCapturedDeclRefType(...) was
missing. This PR fixes that behavior and adds the related diagnostics
too.

This fixes llvm/llvm-project#95081.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepts-invalid clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party lambda C++11 lambda expressions
Projects
Status: Done
5 participants