Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ Improvements to Clang's diagnostics
potential misaligned members get processed before they can get discarded.
(#GH144729)

- Clang now emits dignostic with correct message in case of assigning to const reference captured in lambda. (#GH105647)

Improvements to Clang's time-trace
----------------------------------

Expand Down
2 changes: 2 additions & 0 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13521,6 +13521,8 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {
VarDecl *Var = dyn_cast<VarDecl>(Value);
if (!Var)
return NCCK_None;
if (Var->getType()->isReferenceType())
return NCCK_None;

assert(Var->hasLocalStorage() && "capture added 'const' to non-local?");

Expand Down
5 changes: 5 additions & 0 deletions clang/test/SemaCXX/lambda-expressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ namespace ModifyingCapture {
[=] {
n = 1; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}}
};
const int cn = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should have also tested a constexpr variable case and maybe a auto & assigned from cn as well just to cover a more complete test set.

// cxx03-cxx11-warning@+1 {{initialized lambda captures are a C++14 extension}}
[&cnr = cn]{ // expected-note {{variable 'cnr' declared const here}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note emitted for this line is technically correct, because according to https://eel.is/c++draft/expr.prim.lambda#capture-6, this is where we deduce type of cnr to be const int&. But I believe it would be significantly more helpful if we point out to a const in the declaration of cn, because that's the real reason we can't perform the assignment.

Copy link
Contributor Author

@nfrmtk nfrmtk Aug 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello.
I've thought of this too.
IMO the change shouldn't be limited to lambdas, but to diagnosing attempts to mutate objects behind const references in general.
I think, the most precise (if this word makes any sense in the context) solution is to point to the first place where user added const to type. For example

int a = 5;
const auto& ar = a;
auto& arr = ar;
auto& arrr = arr;
arrr = 2;

Should make note pointing to second line(currently points to fifth).
But I'm not sure if this is common case and will be helpful to the final user.

So, I'm confused and need some help.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to go this far and look through explicit declarations with deduced type.
CC @cor3ntin

cnr = 1; // expected-error {{cannot assign to variable 'cnr' with const-qualified type 'const int &'}}
};
}
}

Expand Down