Skip to content

Commit

Permalink
[Clang] Prevent assignment to captured structured bindings inside imm…
Browse files Browse the repository at this point in the history
…utable 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.
  • Loading branch information
TilakChad authored Dec 25, 2024
1 parent ff97daa commit 70965ef
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 17 deletions.
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,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
} ();
}
}

0 comments on commit 70965ef

Please sign in to comment.