[SEMA] Added warn_decl_shadow support for structured bindings

https://bugs.llvm.org/show_bug.cgi?id=40858

CheckShadow is now called for each binding in the structured binding to make sure it does not shadow any other variable in scope. This does use a custom implementation of getShadowedDeclaration though because a BindingDecl is not a VarDecl

Added a few unit tests for this. In theory though all the other shadow unit tests should be duplicated for the structured binding variables too but whether it is probably not worth it as they use common code. The MyTuple and std interface code has been copied from live-bindings-test.cpp

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D96147

clang/docs/ReleaseNotes.rst

@@ -71,7 +71,7 @@ Deprecated Compiler Flags
 Modified Compiler Flags
 -----------------------
 
-- ...
+- -Wshadow now also checks for shadowed structured bindings
 
 Removed Compiler Flags
 -------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -425,7 +425,8 @@ def warn_decl_shadow :
           "static data member of %2|"
           "field of %2|"
           "typedef in %2|"
-          "type alias in %2}1">,
+          "type alias in %2|"
+          "structured binding}1">,
   InGroup<Shadow>, DefaultIgnore;
 def warn_decl_shadow_uncaptured_local :
   Warning<warn_decl_shadow.Text>,

clang/include/clang/Sema/Sema.h

@@ -2598,6 +2598,8 @@ public:
   NamedDecl *getShadowedDeclaration(const TypedefNameDecl *D,
                                     const LookupResult &R);
   NamedDecl *getShadowedDeclaration(const VarDecl *D, const LookupResult &R);
+  NamedDecl *getShadowedDeclaration(const BindingDecl *D,
+                                    const LookupResult &R);
   void CheckShadow(NamedDecl *D, NamedDecl *ShadowedDecl,
                    const LookupResult &R);
   void CheckShadow(Scope *S, VarDecl *D);

clang/lib/Sema/SemaDecl.cpp

@@ -7491,7 +7491,8 @@ enum ShadowedDeclKind {
   SDK_StaticMember,
   SDK_Field,
   SDK_Typedef,
-  SDK_Using
+  SDK_Using,
+  SDK_StructuredBinding
 };
 
 /// Determine what kind of declaration we're shadowing.
@@ -7501,6 +7502,8 @@ static ShadowedDeclKind computeShadowedDeclKind(const NamedDecl *ShadowedDecl,
     return SDK_Using;
   else if (isa<TypedefDecl>(ShadowedDecl))
     return SDK_Typedef;
+  else if (isa<BindingDecl>(ShadowedDecl))
+    return SDK_StructuredBinding;
   else if (isa<RecordDecl>(OldDC))
     return isa<FieldDecl>(ShadowedDecl) ? SDK_Field : SDK_StaticMember;
 
@@ -7540,9 +7543,8 @@ NamedDecl *Sema::getShadowedDeclaration(const VarDecl *D,
     return nullptr;
 
   NamedDecl *ShadowedDecl = R.getFoundDecl();
-  return isa<VarDecl>(ShadowedDecl) || isa<FieldDecl>(ShadowedDecl)
-             ? ShadowedDecl
-             : nullptr;
+  return isa<VarDecl, FieldDecl, BindingDecl>(ShadowedDecl) ? ShadowedDecl
+                                                            : nullptr;
 }
 
 /// Return the declaration shadowed by the given typedef \p D, or null
@@ -7560,6 +7562,18 @@ NamedDecl *Sema::getShadowedDeclaration(const TypedefNameDecl *D,
   return isa<TypedefNameDecl>(ShadowedDecl) ? ShadowedDecl : nullptr;
 }
 
+/// Return the declaration shadowed by the given variable \p D, or null
+/// if it doesn't shadow any declaration or shadowing warnings are disabled.
+NamedDecl *Sema::getShadowedDeclaration(const BindingDecl *D,
+                                        const LookupResult &R) {
+  if (!shouldWarnIfShadowedDecl(Diags, R))
+    return nullptr;
+
+  NamedDecl *ShadowedDecl = R.getFoundDecl();
+  return isa<VarDecl, FieldDecl, BindingDecl>(ShadowedDecl) ? ShadowedDecl
+                                                            : nullptr;
+}
+
 /// Diagnose variable or built-in function shadowing.  Implements
 /// -Wshadow.
 ///

clang/lib/Sema/SemaDeclCXX.cpp

@@ -857,17 +857,25 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
       Previous.clear();
     }
 
+    auto *BD = BindingDecl::Create(Context, DC, B.NameLoc, B.Name);
+
+    // Find the shadowed declaration before filtering for scope.
+    NamedDecl *ShadowedDecl = D.getCXXScopeSpec().isEmpty()
+                                  ? getShadowedDeclaration(BD, Previous)
+                                  : nullptr;
+
     bool ConsiderLinkage = DC->isFunctionOrMethod() &&
                            DS.getStorageClassSpec() == DeclSpec::SCS_extern;
     FilterLookupForScope(Previous, DC, S, ConsiderLinkage,
                          /*AllowInlineNamespace*/false);
+
     if (!Previous.empty()) {
       auto *Old = Previous.getRepresentativeDecl();
       Diag(B.NameLoc, diag::err_redefinition) << B.Name;
       Diag(Old->getLocation(), diag::note_previous_definition);
+    } else if (ShadowedDecl && !D.isRedeclaration()) {
+      CheckShadow(BD, ShadowedDecl, Previous);
     }
-
-    auto *BD = BindingDecl::Create(Context, DC, B.NameLoc, B.Name);
     PushOnScopeChains(BD, S, true);
     Bindings.push_back(BD);
     ParsingInitForAutoVars.insert(BD);

clang/test/SemaCXX/warn-shadow.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -fsyntax-only -std=c++11 -Wshadow-all %s
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++17 -Wshadow-all %s
 
 namespace {
   int i; // expected-note {{previous declaration is here}}
@@ -265,3 +265,44 @@ struct PR24718_2 {
     PR24718_1 // Does not shadow a type.
   };
 };
+
+namespace structured_binding_tests {
+int x; // expected-note {{previous declaration is here}}
+int y; // expected-note {{previous declaration is here}}
+struct S {
+  int a, b;
+};
+
+void test1() {
+  const auto [x, y] = S(); // expected-warning 2 {{declaration shadows a variable in namespace 'structured_binding_tests'}}
+}
+
+void test2() {
+  int a; // expected-note {{previous declaration is here}}
+  bool b; // expected-note {{previous declaration is here}}
+  {
+    auto [a, b] = S(); // expected-warning 2 {{declaration shadows a local variable}}
+  }
+}
+
+class A
+{
+  int m_a; // expected-note {{previous declaration is here}}
+  int m_b; // expected-note {{previous declaration is here}}
+
+  void test3() {
+    auto [m_a, m_b] = S(); // expected-warning 2 {{declaration shadows a field of 'structured_binding_tests::A'}}
+  }
+};
+
+void test4() {
+  const auto [a, b] = S(); // expected-note 3 {{previous declaration is here}}
+  {
+    int a = 4; // expected-warning {{declaration shadows a structured binding}}
+  }
+  {
+    const auto [a, b] = S(); // expected-warning 2 {{declaration shadows a structured binding}}
+  }
+}
+
+}; // namespace structured_binding_tests