r190382 - Make -Wunused warning rules more consistent.
NAKAMURA Takumi
geek4civic at gmail.com
Tue Sep 10 09:39:11 PDT 2013
Eli, could you work for making llvm/clang tree as clean to new-Wunused?
I was not certain to prune lines according to new warnings.
http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/2184/steps/build/logs/warnings%20%2874%29
2013/9/10 Eli Friedman <eli.friedman at gmail.com>:
> 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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list