r190382 - Make -Wunused warning rules more consistent.
Eli Friedman
eli.friedman at gmail.com
Mon Sep 9 20:05:56 PDT 2013
Author: efriedma
Date: Mon Sep 9 22:05:56 2013
New Revision: 190382
URL: http://llvm.org/viewvc/llvm-project?rev=190382&view=rev
Log:
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>.
Modified:
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h
cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
Modified: cfe/trunk/lib/Sema/Sema.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190382&r1=190381&r2=190382&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.cpp (original)
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Sep 9 22:05:56 2013
@@ -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();
}
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190382&r1=190381&r2=190382&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 9 22:05:56 2013
@@ -1177,6 +1177,14 @@ bool Sema::mightHaveNonExternalLinkage(c
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::ShouldWarnIfUnusedFileScopedD
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::ShouldWarnIfUnusedFileScopedD
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;
}
Modified: cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h?rev=190382&r1=190381&r2=190382&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h (original)
+++ cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h Mon Sep 9 22:05:56 2013
@@ -7,5 +7,7 @@ class A {};
class B {
static A a;
+ static A b;
+ static const int x = sizeof(b);
};
}
Modified: cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190382&r1=190381&r2=190382&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Mon Sep 9 22:05:56 2013
@@ -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
More information about the cfe-commits
mailing list