<div dir="ltr">On 10 September 2013 14:10, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: efriedma<br>
Date: Tue Sep 10 16:10:25 2013<br>
New Revision: 190437<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=190437&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=190437&view=rev</a><br>
Log:<br>
Fix regression from r190382.<br>
<br>
Make sure we perform the correct "referenced-but-not-used" check for<br>
static member constants.<br>
<br>
Fixes bug reported on cfe-commits by Alexey Samsonov.<br></blockquote><div><br></div><div>This breaks -Werror clang bootstrap. Eli, could you help triage? The main problem looks like this:</div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px">
<span style="font-weight:bold">lib/Transforms/Scalar/EarlyCSE.cpp:77:21: </span><span style="font-weight:bold;color:rgb(102,0,0)">error: </span><span style="font-weight:bold">unused variable 'value' [-Werror,-Wunused-variable]</span>
static const bool value = true;
<span style="font-weight:bold;color:rgb(0,128,0)"> ^
</span><span style="font-weight:bold">lib/Transforms/Scalar/EarlyCSE.cpp:225:23: </span><span style="font-weight:bold;color:rgb(102,0,0)">error: </span><span style="font-weight:bold">unused variable 'value' [-Werror,-Wunused-variable]</span>
static const bool value = true;
<span style="font-weight:bold;color:rgb(0,128,0)"> ^</span></pre></div><div>I'm not sure whether this counts as a true positive or not? Is the problem that the template is never instantiated?</div>
<div><br></div><div>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.</div><div><br></div><div>The others are various:</div>
<div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"><span style="font-weight:bold">lib/CodeGen/RegAllocGreedy.cpp:123:28: </span><span style="font-weight:bold;color:rgb(102,0,0)">error: </span><span style="font-weight:bold">unused variable 'StageName' [-Werror,-Wunused-variable]</span>
static const char *const StageName[];
<span style="font-weight:bold;color:rgb(0,128,0)"> ^</span></pre><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"><span style="font-family:arial;line-height:normal;color:rgb(34,34,34)">StageName is only used in debug builds. I suppose we can fix it with (void)StageName?</span></pre>
</div><div></div></div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"><span style="font-family:arial;font-weight:bold">lib/Transforms/Vectorize/LoopVectorize.cpp:127:23: </span><span style="font-family:arial;font-weight:bold;color:rgb(102,0,0)">error: </span><span style="font-family:arial;font-weight:bold">unused variable 'MaxUnrollFactor' [-Werror,-Wunused-variable]</span>
</pre></div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px">static const unsigned MaxUnrollFactor = 16;
<span style="font-weight:bold;color:rgb(0,128,0)"> ^</span></pre><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"><span style="color:rgb(34,34,34);font-family:arial;line-height:normal">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?</span></pre>
</div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"><span style="font-family:arial;font-weight:bold">lib/CodeGen/SelectionDAG/ResourcePriorityQueue.cpp:392:23: </span><span style="font-family:arial;font-weight:bold;color:rgb(102,0,0)">error: </span><span style="font-family:arial;font-weight:bold">unused variable 'PriorityTwo' [-Werror,-Wunused-variable]</span>
</pre></div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px">static const unsigned PriorityTwo = 100;
<span style="font-weight:bold;color:rgb(0,128,0)"> ^</span></pre></div><div>True positive! I guess we can remove it, and anybody who wants to use PriorityTwo gets to add it back.</div><div><br></div>
<div><span style="line-height:18px;white-space:pre-wrap;font-weight:bold">lib/Rewrite/Frontend/RewriteModernObjC.cpp:61:22: </span><span style="line-height:18px;white-space:pre-wrap;font-weight:bold;color:rgb(102,0,0)">error: </span><span style="line-height:18px;white-space:pre-wrap;font-weight:bold">unused variable 'OBJC_ABI_VERSION' [-Werror,-Wunused-variable]</span><br>
</div><div><pre style="line-height:18px;white-space:pre-wrap;margin-bottom:0px;margin-top:0px;padding:1em 0px"> static const int OBJC_ABI_VERSION = 7;
<span style="font-weight:bold;color:rgb(0,128,0)"> ^</span></pre></div><div>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.<br>
</div><div><br></div><div>Nick</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Modified:<br>
cfe/trunk/lib/Sema/Sema.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/Sema.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190437&r1=190436&r2=190437&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190437&r1=190436&r2=190437&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/Sema.cpp (original)<br>
+++ cfe/trunk/lib/Sema/Sema.cpp Tue Sep 10 16:10:25 2013<br>
@@ -358,6 +358,15 @@ static bool ShouldRemoveFromUnused(Sema<br>
}<br>
<br>
if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {<br>
+ // If a variable usable in constant expressions is referenced,<br>
+ // don't warn if it isn't used: if the value of a variable is required<br>
+ // for the computation of a constant expression, it doesn't make sense to<br>
+ // warn even if the variable isn't odr-used. (isReferenced doesn't<br>
+ // precisely reflect that, but it's a decent approximation.)<br>
+ if (VD->isReferenced() &&<br>
+ VD->isUsableInConstantExpressions(SemaRef->Context))<br>
+ return true;<br>
+<br>
// UnusedFileScopedDecls stores the first declaration.<br>
// The declaration may have become definition so check again.<br>
const VarDecl *DeclToCheck = VD->getDefinition();<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190437&r1=190436&r2=190437&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190437&r1=190436&r2=190437&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 10 16:10:25 2013<br>
@@ -1220,14 +1220,6 @@ bool Sema::ShouldWarnIfUnusedFileScopedD<br>
if (!isMainFileLoc(*this, VD->getLocation()))<br>
return false;<br>
<br>
- // If a variable usable in constant expressions is referenced,<br>
- // don't warn if it isn't used: if the value of a variable is required<br>
- // for the computation of a constant expression, it doesn't make sense to<br>
- // warn even if the variable isn't odr-used. (isReferenced doesn't<br>
- // precisely reflect that, but it's a decent approximation.)<br>
- if (VD->isReferenced() && VD->isUsableInConstantExpressions(Context))<br>
- return false;<br>
-<br>
if (Context.DeclMustBeEmitted(VD))<br>
return false;<br>
<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190437&r1=190436&r2=190437&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190437&r1=190436&r2=190437&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Tue Sep 10 16:10:25 2013<br>
@@ -178,4 +178,13 @@ namespace pr14776 {<br>
auto b = X(); // expected-warning {{unused variable 'b'}}<br>
}<br>
<br>
+namespace UndefinedInternalStaticMember {<br>
+ namespace {<br>
+ struct X {<br>
+ static const unsigned x = 3;<br>
+ int y[x];<br>
+ };<br>
+ }<br>
+}<br>
+<br>
#endif<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>