[clang] [Clang] Prevent assignment to captured structured bindings inside immutable lambda (PR #120849)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 23 11:26:09 PST 2024
https://github.com/TilakChad updated https://github.com/llvm/llvm-project/pull/120849
>From f35f0712ea3fcb8c053e7f0405b606c01450ad57 Mon Sep 17 00:00:00 2001
From: Tilak Chad <tilakchad111 at gmail.com>
Date: Sat, 21 Dec 2024 19:22:37 +0545
Subject: [PATCH 1/2] [Clang] Prevent assignment to captured structured
bindings inside immutable lambda
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaExpr.cpp | 25 ++++++++++++++--------
clang/test/SemaCXX/cxx20-decomposition.cpp | 23 ++++++++++++++++++++
3 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6b9e1109f3906e..8b79ab4f551a8a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -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
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 24f7d27c691154..b5e247644ef36e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -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() &&
@@ -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)) {
@@ -13299,7 +13293,18 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) {
// 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) {
+ // Bindings also can be captured by lambda in C++
+ BindingDecl *binding = dyn_cast<BindingDecl>(DRE->getDecl());
+ if (!binding || binding->getType().isConstQualified())
+ return NCCK_None;
+
+ assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?");
+ assert(!isa<BlockDecl>(binding->getDeclContext()));
+
+ return NCCK_Lambda;
+ }
+
if (var->getType().isConstQualified()) return NCCK_None;
assert(var->hasLocalStorage() && "capture added 'const' to non-local?");
@@ -19247,6 +19252,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;
diff --git a/clang/test/SemaCXX/cxx20-decomposition.cpp b/clang/test/SemaCXX/cxx20-decomposition.cpp
index 430a158ff458ed..ccc1af5898059f 100644
--- a/clang/test/SemaCXX/cxx20-decomposition.cpp
+++ b/clang/test/SemaCXX/cxx20-decomposition.cpp
@@ -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
+ } ();
+ }
+}
>From fda25e8e97ffb6f5038c73e204114e81fc862c5c Mon Sep 17 00:00:00 2001
From: Tilak Chad <tilakchad111 at gmail.com>
Date: Tue, 24 Dec 2024 00:55:56 +0545
Subject: [PATCH 2/2] [Clang] Refactored to use common code and updated
variables name
---
clang/lib/Sema/SemaExpr.cpp | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index b5e247644ef36e..562c98c6babe04 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13291,22 +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) {
- // Bindings also can be captured by lambda in C++
- BindingDecl *binding = dyn_cast<BindingDecl>(DRE->getDecl());
- if (!binding || binding->getType().isConstQualified())
- return NCCK_None;
+ ValueDecl *Value = dyn_cast<ValueDecl>(DRE->getDecl());
- assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?");
- assert(!isa<BlockDecl>(binding->getDeclContext()));
+ // 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;
}
- if (var->getType().isConstQualified()) return NCCK_None;
- assert(var->hasLocalStorage() && "capture added 'const' to non-local?");
+ 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;
@@ -13315,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);
}
More information about the cfe-commits
mailing list