r190382 - Make -Wunused warning rules more consistent.
Matt Beaumont-Gay
matthewbg at google.com
Tue Sep 10 14:39:34 PDT 2013
For posterity, ITYM r190437. (I almost pasted r190382 again.)
On Tue, Sep 10, 2013 at 2:26 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> Hopefully r190382 fixes most of the issues.
>
> -Eli
>
>
> On Tue, Sep 10, 2013 at 9:39 AM, NAKAMURA Takumi <geek4civic at gmail.com>
> wrote:
>>
>> 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
>
>
>
> _______________________________________________
> 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