[clang] [Clang] Prevent assignment to captured structured bindings inside immutable lambda (PR #120849)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: TilakChad (TilakChad)

<details>
<summary>Changes</summary>

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

This fixes https://github.com/llvm/llvm-project/issues/95081.

---
Full diff: https://github.com/llvm/llvm-project/pull/120849.diff


3 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+16-9) 
- (modified) clang/test/SemaCXX/cxx20-decomposition.cpp (+23) 


``````````diff
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
+        } ();
+    }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/120849