[clang][Sema] Fix false positive -Wshadow with structured binding captures (llvm#157667)

Previously, lambda init captures of structured bindings were incorrectly
classified as regular shadow warnings (shown with `-Wshadow`), while
regular parameter captures were correctly classified as
`uncaptured-local` warnings (shown only with `-Wshadow-all`). This
created inconsistent behavior:

```cpp
  void foo1(std::pair<int, int> val) {
    [val = std::move(val)](){}();  // No warning with -Wshadow (correct)
  }

  void foo2(std::pair<int, int> val) {
    auto [a, b] = val;
    [a = std::move(a)](){}();      // Warning with -Wshadow (incorrect)
  }
```

The fix extends the existing lambda capture classification logic in
`CheckShadow()` to handle `BindingDecl` consistently with `VarDecl`,
ensuring both cases show no warnings with `-Wshadow` and
`uncaptured-local` warnings with `-Wshadow-all`.

Fixes llvm#68605.

---------

Co-authored-by: Mariya Podchishchaeva <[email protected]>

Author: ivanmurashko

clang/docs/ReleaseNotes.rst

@@ -316,6 +316,11 @@ Bug Fixes in This Version
 - Builtin elementwise operators now accept vector arguments that have different
   qualifiers on their elements. For example, vector of 4 ``const float`` values
   and vector of 4 ``float`` values. (#GH155405)
+- Fixed inconsistent shadow warnings for lambda capture of structured bindings.
+  Previously, ``[val = val]`` (regular parameter) produced no warnings with ``-Wshadow``
+  while ``[a = a]`` (where ``a`` is from ``auto [a, b] = std::make_pair(1, 2)``) 
+  incorrectly produced warnings. Both cases now consistently show no warnings with 
+  ``-Wshadow`` and show uncaptured-local warnings with ``-Wshadow-all``. (#GH68605)
 - Fixed a failed assertion with a negative limit parameter value inside of
   ``__has_embed``. (#GH157842)
 

clang/lib/Sema/SemaDecl.cpp

@@ -8395,7 +8395,7 @@ static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl,
 /// Return the location of the capture if the given lambda captures the given
 /// variable \p VD, or an invalid source location otherwise.
 static SourceLocation getCaptureLocation(const LambdaScopeInfo *LSI,
-                                         const VarDecl *VD) {
+                                         const ValueDecl *VD) {
   for (const Capture &Capture : LSI->Captures) {
     if (Capture.isVariableCapture() && Capture.getVariable() == VD)
       return Capture.getLocation();
@@ -8492,7 +8492,9 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
   if (isa<VarDecl>(D) && NewDC && isa<CXXMethodDecl>(NewDC)) {
     if (const auto *RD = dyn_cast<CXXRecordDecl>(NewDC->getParent())) {
       if (RD->isLambda() && OldDC->Encloses(NewDC->getLexicalParent())) {
-        if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl)) {
+        // Handle both VarDecl and BindingDecl in lambda contexts
+        if (isa<VarDecl, BindingDecl>(ShadowedDecl)) {
+          const auto *VD = cast<ValueDecl>(ShadowedDecl);
           const auto *LSI = cast<LambdaScopeInfo>(getCurFunction());
           if (RD->getLambdaCaptureDefault() == LCD_None) {
             // Try to avoid warnings for lambdas with an explicit capture
@@ -8521,18 +8523,27 @@ void Sema::CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
           return;
         }
       }
-      if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl);
-          VD && VD->hasLocalStorage()) {
-        // A variable can't shadow a local variable in an enclosing scope, if
-        // they are separated by a non-capturing declaration context.
-        for (DeclContext *ParentDC = NewDC;
-             ParentDC && !ParentDC->Equals(OldDC);
-             ParentDC = getLambdaAwareParentOfDeclContext(ParentDC)) {
-          // Only block literals, captured statements, and lambda expressions
-          // can capture; other scopes don't.
-          if (!isa<BlockDecl>(ParentDC) && !isa<CapturedDecl>(ParentDC) &&
-              !isLambdaCallOperator(ParentDC)) {
-            return;
+      // Apply scoping logic to both VarDecl and BindingDecl with local storage
+      if (isa<VarDecl, BindingDecl>(ShadowedDecl)) {
+        bool HasLocalStorage = false;
+        if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl))
+          HasLocalStorage = VD->hasLocalStorage();
+        else if (const auto *BD = dyn_cast<BindingDecl>(ShadowedDecl))
+          HasLocalStorage =
+              cast<VarDecl>(BD->getDecomposedDecl())->hasLocalStorage();
+
+        if (HasLocalStorage) {
+          // A variable can't shadow a local variable or binding in an enclosing
+          // scope, if they are separated by a non-capturing declaration
+          // context.
+          for (DeclContext *ParentDC = NewDC;
+               ParentDC && !ParentDC->Equals(OldDC);
+               ParentDC = getLambdaAwareParentOfDeclContext(ParentDC)) {
+            // Only block literals, captured statements, and lambda expressions
+            // can capture; other scopes don't.
+            if (!isa<BlockDecl>(ParentDC) && !isa<CapturedDecl>(ParentDC) &&
+                !isLambdaCallOperator(ParentDC))
+              return;
           }
         }
       }
@@ -8579,7 +8590,8 @@ void Sema::DiagnoseShadowingLambdaDecls(const LambdaScopeInfo *LSI) {
     const NamedDecl *ShadowedDecl = Shadow.ShadowedDecl;
     // Try to avoid the warning when the shadowed decl isn't captured.
     const DeclContext *OldDC = ShadowedDecl->getDeclContext();
-    if (const auto *VD = dyn_cast<VarDecl>(ShadowedDecl)) {
+    if (isa<VarDecl, BindingDecl>(ShadowedDecl)) {
+      const auto *VD = cast<ValueDecl>(ShadowedDecl);
       SourceLocation CaptureLoc = getCaptureLocation(LSI, VD);
       Diag(Shadow.VD->getLocation(),
            CaptureLoc.isInvalid() ? diag::warn_decl_shadow_uncaptured_local

clang/test/SemaCXX/PR68605.cpp

@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++20 -Wshadow %s
+// RUN: %clang_cc1 -verify=all -fsyntax-only -std=c++20 -Wshadow-all %s
+
+// Test for issue #68605: Inconsistent shadow warnings for lambda capture of structured bindings.
+// 
+// The issue was that structured binding lambda captures were incorrectly classified
+// as regular shadow warnings (shown with -Wshadow) while regular parameter captures 
+// were classified as uncaptured-local warnings (shown only with -Wshadow-all).
+//
+// This test validates that both VarDecl and BindingDecl lambda captures now 
+// behave consistently: no warnings with -Wshadow, but uncaptured-local warnings 
+// with -Wshadow-all.
+
+namespace std {
+  template<typename T> T&& move(T&& t) { return static_cast<T&&>(t); }
+}
+
+namespace issue_68605 {
+
+// Simple pair-like struct for testing
+struct Pair {
+  int first;
+  int second;
+  Pair(int f, int s) : first(f), second(s) {}
+};
+
+// Test case 1: Regular parameter - consistent behavior
+void foo1(Pair val) { // all-note {{previous declaration is here}}
+  [val = std::move(val)](){}(); // all-warning {{declaration shadows a local variable}}
+}
+
+// Test case 2: Structured binding - now consistent with regular parameter
+void foo2(Pair val) {
+  auto [a,b] = val; // all-note {{previous declaration is here}}
+  [a = std::move(a)](){}(); // all-warning {{declaration shadows a structured binding}}
+}
+
+// Test case 3: Multiple captures showing consistent behavior
+void foo3() {
+  Pair data{42, 100};
+  auto [id, value] = data; // all-note 2{{previous declaration is here}}
+  
+  // Both show consistent uncaptured-local warnings with -Wshadow-all
+  auto lambda1 = [id = id](){ return id; }; // all-warning {{declaration shadows a structured binding}}
+  auto lambda2 = [value = value](){ return value; }; // all-warning {{declaration shadows a structured binding}}
+}
+
+// Test case 4: Mixed scenario showing consistent behavior
+void foo4() {
+  int regular_var = 10; // all-note {{previous declaration is here}}
+  Pair pair_data{1, 2};
+  auto [x, y] = pair_data; // all-note 2{{previous declaration is here}}
+  
+  // All captures now show consistent uncaptured-local warnings with -Wshadow-all
+  auto lambda1 = [regular_var = regular_var](){}; // all-warning {{declaration shadows a local variable}}
+  auto lambda2 = [x = x](){}; // all-warning {{declaration shadows a structured binding}}
+  auto lambda3 = [y = y](){}; // all-warning {{declaration shadows a structured binding}}
+}
+
+// Test case 5: Ensure we don't break existing shadow detection for actual shadowing
+void foo5() {
+  int outer = 5; // expected-note {{previous declaration is here}} all-note {{previous declaration is here}}
+  auto [a, b] = Pair{1, 2}; // expected-note {{previous declaration is here}} all-note {{previous declaration is here}}
+  
+  // This SHOULD still warn - it's actual shadowing within the lambda body
+  auto lambda = [outer, a](){ // expected-note {{variable 'outer' is explicitly captured here}} all-note {{variable 'outer' is explicitly captured here}} expected-note {{variable 'a' is explicitly captured here}} all-note {{variable 'a' is explicitly captured here}}
+    int outer = 10; // expected-warning {{declaration shadows a local variable}} all-warning {{declaration shadows a local variable}}
+    int a = 20;     // expected-warning {{declaration shadows a structured binding}} all-warning {{declaration shadows a structured binding}}
+  };
+}
+
+} // namespace issue_68605

clang/test/SemaCXX/warn-shadow-in-lambdas.cpp

@@ -258,10 +258,15 @@ struct S {
 };
 
 int foo() {
-  auto [a] = S{0}; // expected-note {{previous}} \
-                   // cxx14-warning {{decomposition declarations are a C++17 extension}}
+#ifdef AVOID
+  auto [a] = S{0}; // cxx14-warning {{decomposition declarations are a C++17 extension}}
+  [a = a] () { // No warning with basic -Wshadow due to uncaptured-local classification
+  }();
+#else
+  auto [a] = S{0}; // cxx14-warning {{decomposition declarations are a C++17 extension}} expected-note {{previous declaration is here}}
   [a = a] () { // expected-warning {{declaration shadows a structured binding}}
   }();
+#endif
 }
 
 }