r190437 - Fix regression from r190382.

[Nick Lewycky]

On 10 September 2013 14:10, Eli Friedman <eli.friedman at gmail.com> wrote:

> Author: efriedma
> Date: Tue Sep 10 16:10:25 2013
> New Revision: 190437
>
> URL: http://llvm.org/viewvc/llvm-project?rev=190437&view=rev
> Log:
> Fix regression from r190382.
>
> Make sure we perform the correct "referenced-but-not-used" check for
> static member constants.
>
> Fixes bug reported on cfe-commits by Alexey Samsonov.
>

This breaks -Werror clang bootstrap. Eli, could you help triage? The main
problem looks like this:

lib/Transforms/Scalar/EarlyCSE.cpp:77:21: error: unused variable
'value' [-Werror,-Wunused-variable]
  static const bool value = true;
^lib/Transforms/Scalar/EarlyCSE.cpp:225:23: error: unused variable
'value' [-Werror,-Wunused-variable]
    static const bool value = true;                      ^

I'm not sure whether this counts as a true positive or not? Is the problem
that the template is never instantiated?

This sort of thing occurs here,
lib/Transforms/InstCombine/InstCombinePHI.cpp:608,
tools/clang/lib/Sema/SemaType.cpp:256 and
lib/Rewrite/Frontend/RewriteModernObjC.cpp:61.

The others are various:

lib/CodeGen/RegAllocGreedy.cpp:123:28: error: unused variable
'StageName' [-Werror,-Wunused-variable]
  static const char *const StageName[];                           ^

StageName is only used in debug builds. I suppose we can fix it with
(void)StageName?

lib/Transforms/Vectorize/LoopVectorize.cpp:127:23: error: unused
variable 'MaxUnrollFactor' [-Werror,-Wunused-variable]

static const unsigned MaxUnrollFactor = 16;                      ^

This is another example of "only used in debug" but I'm not sure
what's up. If MaxUnrollFactor is only used in an assert what makes it
true that the condition it's used in always holds?

lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp:392:23: error:
unused variable 'PriorityTwo' [-Werror,-Wunused-variable]

static const unsigned PriorityTwo = 100;                      ^

True positive! I guess we can remove it, and anybody who wants to use
PriorityTwo gets to add it back.

lib/Rewrite/Frontend/RewriteModernObjC.cpp:61:22: error: unused variable
'OBJC_ABI_VERSION' [-Werror,-Wunused-variable]

    static const int OBJC_ABI_VERSION = 7;                     ^

Another true positive. It's in a struct, but the struct is itself in an
anonymous namespace, so we can safely remove any dead members.

Nick


> Modified:
>     cfe/trunk/lib/Sema/Sema.cpp
>     cfe/trunk/lib/Sema/SemaDecl.cpp
>     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=190437&r1=190436&r2=190437&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Tue Sep 10 16:10:25 2013
> @@ -358,6 +358,15 @@ static bool ShouldRemoveFromUnused(Sema
>    }
>
>    if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
> +    // 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(SemaRef->Context))
> +      return true;
> +
>      // UnusedFileScopedDecls stores the first declaration.
>      // The declaration may have become definition so check again.
>      const VarDecl *DeclToCheck = VD->getDefinition();
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190437&r1=190436&r2=190437&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 10 16:10:25 2013
> @@ -1220,14 +1220,6 @@ bool Sema::ShouldWarnIfUnusedFileScopedD
>      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;
>
>
> 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=190437&r1=190436&r2=190437&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Tue Sep 10 16:10:25
> 2013
> @@ -178,4 +178,13 @@ namespace pr14776 {
>    auto b = X(); // expected-warning {{unused variable 'b'}}
>  }
>
> +namespace UndefinedInternalStaticMember {
> +  namespace {
> +    struct X {
> +      static const unsigned x = 3;
> +      int y[x];
> +    };
> +  }
> +}
> +
>  #endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130910/9e9ff8d9/attachment.html>