From: Eli Friedman Date: Tue, 10 Sep 2013 03:05:56 +0000 (+0000) Subject: Make -Wunused warning rules more consistent. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=39bd371610af27b073c792c54c6c28133329f6ad;p=clang 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 . 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 71d726f619..fbff9499d5 100644 --- 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 34f7c46b67..36105e7117 100644 --- 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(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 5fac45922c..56990e0a33 100644 --- 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 9fb601130d..aa2b2e5103 100644 --- 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 + 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 + static inline void bar(T, U) { } + + template + 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 - 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 - static inline void bar(T, U) { } - - template - 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