r190382 - Make -Wunused warning rules more consistent.
Alexey Samsonov
samsonov at google.com
Tue Sep 10 05:28:03 PDT 2013
Hi Eli,
Recently sanitizer-x86_64-linux buildbot which bootstraps Clang started
failing with -Wunused warnings which looks like false positives to me:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/848/steps/build%2064-bit%20llvm%20using%20clang/logs/stdio
/build/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp:108:23:
error: variable 'kNumberOfAccessSizes' is not needed and will not be
emitted [-Werror,-Wunneeded-internal-declaration]
static const size_t kNumberOfAccessSizes = 5;
This static member is actually used to define array sizes, and in the
function code as well.
Can this be related to your change?
On Tue, Sep 10, 2013 at 7:05 AM, Eli Friedman <eli.friedman at gmail.com>wrote:
> 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
>
--
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130910/2b3b5509/attachment.html>
More information about the cfe-commits
mailing list