Make -Wunused warning rules more consistent.

This patch does a few different things.

This patch improves unused var diags for const vars: we no longer
unconditionally suppress diagnostics for const vars, instead only suppressing
the diagnostic when the declaration appears to be useful.

This patch also makes us more consistently use whether a variable/function
is declared in the main file to suppress diagnostics where appropriate.

Fixes <rdar://problem/14907887>.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190382 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 71d726f61956b2b0bbd21d55e4ae70db7653b433..fbff9499d53a37b13fe8155011dbd3bbd36a943a 100644 (file)
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -749,11 +749,7 @@ void Sema::ActOnEndOfTranslationUnit() {
         if (DiagD->isReferenced()) {
           Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
                 << /*variable*/1 << DiagD->getDeclName();
-        } else if (SourceMgr.isInMainFile(DiagD->getLocation())) {
-          // If the declaration is in a header which is included into multiple
-          // TUs, it will declare one variable per TU, and one of the other
-          // variables may be used. So, only warn if the declaration is in the
-          // main file.
+        } else {
           Diag(DiagD->getLocation(), diag::warn_unused_variable)
               << DiagD->getDeclName();
         }

diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 34f7c46b67593c3e68228ae365d95a78316a78cb..36105e7117c1d85b960fbd055ae3838d5c724d0e 100644 (file)
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1177,6 +1177,14 @@ bool Sema::mightHaveNonExternalLinkage(const DeclaratorDecl *D) {
   return !D->isExternallyVisible();
 }
 
+// FIXME: This needs to be refactored; some other isInMainFile users want
+// these semantics.
+static bool isMainFileLoc(const Sema &S, SourceLocation Loc) {
+  if (S.TUKind != TU_Complete)
+    return false;
+  return S.SourceMgr.isInMainFile(Loc);
+}
+
 bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
   assert(D);
 
@@ -1196,12 +1204,9 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
       if (MD->isVirtual() || IsDisallowedCopyOrAssign(MD))
         return false;
     } else {
-      // 'static inline' functions are used in headers; don't warn.
-      // Make sure we get the storage class from the canonical declaration,
-      // since otherwise we will get spurious warnings on specialized
-      // static template functions.
-      if (FD->getCanonicalDecl()->getStorageClass() == SC_Static &&
-          FD->isInlineSpecified())
+      // 'static inline' functions are defined in headers; don't warn.
+      if (FD->isInlineSpecified() &&
+          !isMainFileLoc(*this, FD->getLocation()))
         return false;
     }
 
@@ -1209,21 +1214,26 @@ bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {
         Context.DeclMustBeEmitted(FD))
       return false;
   } else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
-    // Don't warn on variables of const-qualified or reference type, since their
-    // values can be used even if though they're not odr-used, and because const
-    // qualified variables can appear in headers in contexts where they're not
-    // intended to be used.
-    // FIXME: Use more principled rules for these exemptions.
-    if (!VD->isFileVarDecl() ||
-        VD->getType().isConstQualified() ||
-        VD->getType()->isReferenceType() ||
-        Context.DeclMustBeEmitted(VD))
+    // Constants and utility variables are defined in headers with internal
+    // linkage; don't warn.  (Unlike functions, there isn't a convenient marker
+    // like "inline".)
+    if (!isMainFileLoc(*this, VD->getLocation()))
+      return false;
+
+    // If a variable usable in constant expressions is referenced,
+    // don't warn if it isn't used: if the value of a variable is required
+    // for the computation of a constant expression, it doesn't make sense to
+    // warn even if the variable isn't odr-used.  (isReferenced doesn't
+    // precisely reflect that, but it's a decent approximation.)
+    if (VD->isReferenced() && VD->isUsableInConstantExpressions(Context))
+      return false;
+
+    if (Context.DeclMustBeEmitted(VD))
       return false;
 
     if (VD->isStaticDataMember() &&
         VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
       return false;
-
   } else {
     return false;
   }

diff --git a/test/SemaCXX/Inputs/warn-unused-variables.h b/test/SemaCXX/Inputs/warn-unused-variables.h
index 5fac45922c99ccd6cf62451c7e0c81b4a25cecda..56990e0a33de7e8c0fae9a5dcc64819c1c6ad06f 100644 (file)
--- a/test/SemaCXX/Inputs/warn-unused-variables.h
+++ b/test/SemaCXX/Inputs/warn-unused-variables.h
@@ -7,5 +7,7 @@ class A {};
 
 class B {
   static A a;
+  static A b;
+  static const int x = sizeof(b);
 };
 }

diff --git a/test/SemaCXX/warn-unused-filescoped.cpp b/test/SemaCXX/warn-unused-filescoped.cpp
index 9fb601130d3b5e1da949dbbc56f1cdbb8032a675..aa2b2e5103cb6e92f55c8a68ab79e834acefaf6d 100644 (file)
--- a/test/SemaCXX/warn-unused-filescoped.cpp
+++ b/test/SemaCXX/warn-unused-filescoped.cpp
@@ -1,6 +1,41 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -Wno-c++11-extensions -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -std=c++11 %s
 
+#ifdef HEADER
+
+static void headerstatic() {}  // expected-warning{{unused}}
+static inline void headerstaticinline() {}
+
+namespace {
+  void headeranon() {}  // expected-warning{{unused}}
+  inline void headerinlineanon() {}
+}
+
+namespace test7
+{
+  template<typename T>
+  static inline void foo(T) { }
+
+  // This should not emit an unused-function warning since it inherits
+  // the static storage type from the base template.
+  template<>
+  inline void foo(int) {  }
+
+  // Partial specialization
+  template<typename T, typename U>
+  static inline void bar(T, U) { }
+
+  template<typename U>
+  inline void bar(int, U) { }
+
+  template<>
+  inline void bar(int, int) { }
+};
+
+#else
+#define HEADER
+#include "warn-unused-filescoped.cpp"
+
 static void f1(); // expected-warning{{unused}}
 
 namespace {
@@ -37,8 +72,10 @@ namespace {
 
 void S::m3() { }  // expected-warning{{unused}}
 
-static inline void f4() { }
-const unsigned int cx = 0;
+static inline void f4() { }  // expected-warning{{unused}}
+const unsigned int cx = 0; // expected-warning{{unused}}
+const unsigned int cy = 0;
+int f5() { return cy; }
 
 static int x1;  // expected-warning{{unused}}
 
@@ -98,7 +135,7 @@ namespace test5 {
   // FIXME: We should produce warnings for both of these.
   static const int m = n;
   int x = sizeof(m);
-  static const double d = 0.0;
+  static const double d = 0.0; // expected-warning{{not needed and will not be emitted}}
   int y = sizeof(d);
 }
 
@@ -133,27 +170,6 @@ namespace test6 {
   };
 }
 
-namespace test7
-{
-  template<typename T>
-  static inline void foo(T) { }
-
-  // This should not emit an unused-function warning since it inherits
-  // the static storage type from the base template.
-  template<>
-  inline void foo(int) {  }
-
-  // Partial specialization
-  template<typename T, typename U>
-  static inline void bar(T, U) { }
-
-  template<typename U>
-  inline void bar(int, U) { }
-
-  template<>
-  inline void bar(int, int) { }
-};
-
 namespace pr14776 {
   namespace {
     struct X {};
@@ -161,3 +177,5 @@ namespace pr14776 {
   X a = X(); // expected-warning {{unused variable 'a'}}
   auto b = X(); // expected-warning {{unused variable 'b'}}
 }
+
+#endif