Skip to content

[Clang] Prevent assignment to captured structured bindings inside immutable lambda #120849

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

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ Bug Fixes to C++ Support
- Fixed recognition of ``std::initializer_list`` when it's surrounded with ``extern "C++"`` and exported
out of a module (which is the case e.g. in MSVC's implementation of ``std`` module). (#GH118218)
- Fixed a pack expansion issue in checking unexpanded parameter sizes. (#GH17042)
- Fixed a bug where captured structured bindings were modifiable inside non-mutable lambda (#GH95081)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
43 changes: 26 additions & 17 deletions clang/lib/Sema/SemaExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3352,6 +3352,7 @@ ExprResult Sema::BuildDeclarationNameExpr(
case Decl::VarTemplateSpecialization:
case Decl::VarTemplatePartialSpecialization:
case Decl::Decomposition:
case Decl::Binding:
case Decl::OMPCapturedExpr:
// In C, "extern void blah;" is valid and is an r-value.
if (!getLangOpts().CPlusPlus && !type.hasQualifiers() &&
Expand All @@ -3371,20 +3372,13 @@ ExprResult Sema::BuildDeclarationNameExpr(
// potentially-evaluated contexts? Since the variable isn't actually
// captured in an unevaluated context, it seems that the answer is no.
if (!isUnevaluatedContext()) {
QualType CapturedType = getCapturedDeclRefType(cast<VarDecl>(VD), Loc);
QualType CapturedType = getCapturedDeclRefType(cast<ValueDecl>(VD), Loc);
if (!CapturedType.isNull())
type = CapturedType;
}

break;
}

case Decl::Binding:
// These are always lvalues.
valueKind = VK_LValue;
type = type.getNonReferenceType();
break;

case Decl::Function: {
if (unsigned BID = cast<FunctionDecl>(VD)->getBuiltinID()) {
if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) {
Expand Down Expand Up @@ -13297,11 +13291,24 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {
if (!DRE) return NCCK_None;
if (!DRE->refersToEnclosingVariableOrCapture()) return NCCK_None;

// The declaration must be a variable which is not declared 'const'.
VarDecl *var = dyn_cast<VarDecl>(DRE->getDecl());
if (!var) return NCCK_None;
if (var->getType().isConstQualified()) return NCCK_None;
assert(var->hasLocalStorage() && "capture added 'const' to non-local?");
ValueDecl *Value = dyn_cast<ValueDecl>(DRE->getDecl());

// The declaration must be a value which is not declared 'const'.
if (!Value || Value->getType().isConstQualified())
return NCCK_None;

BindingDecl *Binding = dyn_cast<BindingDecl>(Value);
if (Binding) {
assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?");
assert(!isa<BlockDecl>(Binding->getDeclContext()));
return NCCK_Lambda;
}

VarDecl *Var = dyn_cast<VarDecl>(Value);
if (!Var)
return NCCK_None;

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

// Decide whether the first capture was for a block or a lambda.
DeclContext *DC = S.CurContext, *Prev = nullptr;
Expand All @@ -13310,16 +13317,16 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {
// For init-capture, it is possible that the variable belongs to the
// template pattern of the current context.
if (auto *FD = dyn_cast<FunctionDecl>(DC))
if (var->isInitCapture() &&
FD->getTemplateInstantiationPattern() == var->getDeclContext())
if (Var->isInitCapture() &&
FD->getTemplateInstantiationPattern() == Var->getDeclContext())
break;
if (DC == var->getDeclContext())
if (DC == Var->getDeclContext())
break;
Prev = DC;
DC = DC->getParent();
}
// Unless we have an init-capture, we've gone one step too far.
if (!var->isInitCapture())
if (!Var->isInitCapture())
DC = Prev;
return (isa<BlockDecl>(DC) ? NCCK_Block : NCCK_Lambda);
}
Expand Down Expand Up @@ -19247,6 +19254,8 @@ bool Sema::NeedToCaptureVariable(ValueDecl *Var, SourceLocation Loc) {
}

QualType Sema::getCapturedDeclRefType(ValueDecl *Var, SourceLocation Loc) {
assert(Var && "Null value cannot be captured");

QualType CaptureType;
QualType DeclRefType;

Expand Down
23 changes: 23 additions & 0 deletions clang/test/SemaCXX/cxx20-decomposition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,3 +183,26 @@ namespace ODRUseTests {
}(0); }(0); // expected-note 2{{in instantiation}}
}
}


namespace GH95081 {
void prevent_assignment_check() {
int arr[] = {1,2};
auto [e1, e2] = arr;

auto lambda = [e1] {
e1 = 42; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}}
};
}

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

int arr[1];
void foo() {
auto [x] = arr;
[x]() {
f(x); // deleted f(int&) used to be picked up erroneously
} ();
}
}
Loading