PR33839: Fix -Wunused handling for structured binding declarations.

We warn about a structured binding declaration being unused only if none of its
bindings are used.

llvm-svn: 314733

4 files changed, 72 insertions, 10 deletions

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index fa781194f38..fcdc0b39dda 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -1604,7 +1604,24 @@ static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
   if (D->isInvalidDecl())
     return false;
 
-  if (D->isReferenced() || D->isUsed() || D->hasAttr<UnusedAttr>() ||
+  bool Referenced = false;
+  if (auto *DD = dyn_cast<DecompositionDecl>(D)) {
+    // For a decomposition declaration, warn if none of the bindings are
+    // referenced, instead of if the variable itself is referenced (which
+    // it is, by the bindings' expressions).
+    for (auto *BD : DD->bindings()) {
+      if (BD->isReferenced()) {
+        Referenced = true;
+        break;
+      }
+    }
+  } else if (!D->getDeclName()) {
+    return false;
+  } else if (D->isReferenced() || D->isUsed()) {
+    Referenced = true;
+  }
+
+  if (Referenced || D->hasAttr<UnusedAttr>() ||
       D->hasAttr<ObjCPreciseLifetimeAttr>())
     return false;
 
@@ -1727,7 +1744,7 @@ void Sema::DiagnoseUnusedDecl(const NamedDecl *D) {
   else
     DiagID = diag::warn_unused_variable;
 
-  Diag(D->getLocation(), DiagID) << D->getDeclName() << Hint;
+  Diag(D->getLocation(), DiagID) << D << Hint;
 }
 
 static void CheckPoppedLabel(LabelDecl *L, Sema &S) {
@@ -1757,8 +1774,6 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
     assert(isa<NamedDecl>(TmpD) && "Decl isn't NamedDecl?");
     NamedDecl *D = cast<NamedDecl>(TmpD);
 
-    if (!D->getDeclName()) continue;
-
     // Diagnose unused variables in this scope.
     if (!S->hasUnrecoverableErrorOccurred()) {
       DiagnoseUnusedDecl(D);
@@ -1766,6 +1781,8 @@ void Sema::ActOnPopScope(SourceLocation Loc, Scope *S) {
         DiagnoseUnusedNestedTypedefs(RD);
     }
 
+    if (!D->getDeclName()) continue;
+
     // If this was a forward reference to a label, verify it was defined.
     if (LabelDecl *LD = dyn_cast<LabelDecl>(D))
       CheckPoppedLabel(LD, *this);
@@ -6164,7 +6181,6 @@ NamedDecl *Sema::ActOnVariableDeclarator(
   IdentifierInfo *II = Name.getAsIdentifierInfo();
 
   if (D.isDecompositionDeclarator()) {
-    AddToScope = false;
     // Take the name of the first declarator as our name for diagnostic
     // purposes.
     auto &Decomp = D.getDecompositionDeclarator();
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 82e112bb3f9..5d9a39c6772 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -829,7 +829,10 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator &D,
   NamedDecl *New =
       ActOnVariableDeclarator(S, D, DC, TInfo, Previous,
                               MultiTemplateParamsArg(), AddToScope, Bindings);
-  CurContext->addHiddenDecl(New);
+  if (AddToScope) {
+    S->AddDecl(New);
+    CurContext->addHiddenDecl(New);
+  }
 
   if (isInOpenMPDeclareTargetContext())
     checkDeclIsAllowedInOpenMPTarget(nullptr, New);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2c51c57bf39..d7fb6428c4a 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -677,6 +677,7 @@ TemplateDeclInstantiator::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
 Decl *TemplateDeclInstantiator::VisitBindingDecl(BindingDecl *D) {
   auto *NewBD = BindingDecl::Create(SemaRef.Context, Owner, D->getLocation(),
                                     D->getIdentifier());
+  NewBD->setReferenced(D->isReferenced());
   SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewBD);
   return NewBD;
 }
diff --git a/clang/test/SemaCXX/unused.cpp b/clang/test/SemaCXX/unused.cpp
index 09a179e7bb8..ba9ab2363b0 100644
--- a/clang/test/SemaCXX/unused.cpp
+++ b/clang/test/SemaCXX/unused.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wunused %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 -Wunused %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wunused %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 -Wunused %s
 
 // PR4103 : Make sure we don't get a bogus unused expression warning
 namespace PR4103 {
@@ -8,7 +9,7 @@ namespace PR4103 {
     char foo;
   };
   class APSInt : public APInt {
-    char bar;
+    char bar; // expected-warning {{private field 'bar' is not used}}
   public:
     APSInt &operator=(const APSInt &RHS);
   };
@@ -69,3 +70,44 @@ namespace UnresolvedLookup {
     }
   };
 }
+
+#if __cplusplus >= 201703L
+namespace PR33839 {
+  void a() {
+    struct X { int a, b; } x;
+    auto [a, b] = x; // expected-warning {{unused variable '[a, b]'}}
+    auto [c, d] = x;
+    (void)d;
+  }
+
+  template<typename T> void f() {
+    struct A { int n; } a[1];
+    for (auto [x] : a) {
+      (void)x;
+    }
+    auto [y] = a[0]; // expected-warning {{unused}}
+  }
+  template<bool b> void g() {
+    struct A { int n; } a[1];
+    for (auto [x] : a) {
+      if constexpr (b)
+        (void)x;
+    }
+
+    auto [y] = a[0];
+    if constexpr (b)
+      (void)y; // ok, even when b == false
+  }
+  template<typename T> void h() {
+    struct A { int n; } a[1];
+    for (auto [x] : a) { // expected-warning {{unused variable '[x]'}}
+    }
+  }
+  void use() { 
+    f<int>(); // expected-note {{instantiation of}}
+    g<true>();
+    g<false>();
+    h<int>(); // expected-note {{instantiation of}}
+  }
+}
+#endif