<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 26, 2012, at 3:12 PM, Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Fri, Oct 26, 2012 at 2:19 PM, Argyrios Kyrtzidis <<a href="mailto:kyrtzidis@apple.com">kyrtzidis@apple.com</a>> wrote:<br><blockquote type="cite">On Oct 26, 2012, at 2:16 PM, Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:<br><br><blockquote type="cite">FWIW, I found -Wunneeded-internal-declaration fairly confusing. I<br>tried to turn it on for chromium, and it fired in a few instances<br>where the function it warned about was used for something and couldn't<br>be easily removed.<br></blockquote><br>Could you be more specific ? It's not clear if you are talking about a false positive or something else.<br></blockquote><br>I don't remember details. I did a new build with the flag right now,<br>here's what it found:<br><br><br>In this case, clang warns about _NullPool which is used in the<br>function Null() which in turn is used in some translation units:<br><br><a href="http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/harfbuzz-ng/src/hb-open-type-private.hh&exact_package=chromium&q=_NullPool&l=134">http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/harfbuzz-ng/src/hb-open-type-private.hh&exact_package=chromium&q=_NullPool&l=134</a><br>http://code.google.com/p/chromium/source/search?q=file%3Aharfbuzz+Null%5C%28&origq=file%3Aharfbuzz+Null%5C%28&btnG=Search+Trunk<br><br>../../third_party/harfbuzz-ng/src/hb-open-type-private.hh:136:20:<br>error: variable '_NullPool' is not needed and will not be emitted<br>[-Werror,-Wunneeded-internal-declaration]<br>static const void *_NullPool[64 / sizeof (void *)];<br> ^<br>1 error generated.<br><br>I'm not sure what to do with this warning.<br></blockquote><div><br></div><div>It says so in the comment above it :-)</div><div><br></div><div><blockquote type="cite"><pre id="c0" style="margin-top: 0px; margin-bottom: 0px; z-index: 50; position: relative; padding-top: 0.5em; line-height: 16px; background-color: rgb(255, 255, 255); "><span id="c0_135" class="stx-line" style="display: block; position: relative; "><span class="stx-comment" style="color: rgb(136, 0, 0); ">/* TODO This really should be a extern HB_INTERNAL and defined somewhere... */</span>
</span></pre></blockquote><div><br></div><div>We could have the warning suppressed for static variables in headers though.</div><div><br></div></div><blockquote type="cite"><br><br><br>A related example: clang thinks that GrInitialArrayAllocationCount()<br>should be "static inline" (ok, I guess). However,<br>GrNextArrayAllocationCount() right below it doesn't warn, even though<br>the two functions seem to be used in the same way later in the file<br>(in GrTDArray::growAt()).<br><br><a href="http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/skia/src/gpu/GrTDArray.h&exact_package=chromium&q=GrInitialArrayAllocationCount&l=17">http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/skia/src/gpu/GrTDArray.h&exact_package=chromium&q=GrInitialArrayAllocationCount&l=17</a><br><br>In file included from ../../third_party/skia/src/gpu/gl/GrGpuGL.cpp:9:<br>In file included from ../../third_party/skia/src/gpu/gl/GrGpuGL.h:23:<br>In file included from ../../third_party/skia/src/gpu/gl/../GrTHashCache.h:14:<br>../../third_party/skia/src/gpu/GrTDArray.h:17:12: error: 'static'<br>function 'GrInitialArrayAllocationCount' declared in header file<br>should be declared 'static inline'<br>[-Werror,-Wunneeded-internal-declaration]<br>static int GrInitialArrayAllocationCount() {<br> ^<br>1 error generated.<br></blockquote><div><br></div><div>I assume you mean there's a false negative here ? Is there no "unused function" warning ? I suggest filing a bug with a preprocessed file.</div><div><br></div><div>-Argyrios</div><br><blockquote type="cite"><br><br><br>…and that's all already. The last time I tried I think there were more issues.<br><br>Nico<br><br><blockquote type="cite"><br>-Argyrios<br><br><blockquote type="cite"><br>Nico<br><br>On Fri, Oct 26, 2012 at 1:54 PM, Argyrios Kyrtzidis <<a href="mailto:kyrtzidis@apple.com">kyrtzidis@apple.com</a>> wrote:<br><blockquote type="cite">On Oct 26, 2012, at 1:35 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br><br><blockquote type="cite">Argyrios: It looks like you added this warning in r129794. Can you<br>comment on what it's intended to detect?<br></blockquote><br>variables/functions with internal linkage that are not used from the codegen perspective.<br>This differs from -Wunused which will consider a 'use' even in an unevaluated context.<br>For example:<br><br>static void foo() { }<br><br>this gives:<br>warning: unused function 'foo' [-Wunused-function]<br><br>static void foo() { }<br>template <typename T><br>void goo() {<br> foo();<br>}<br><br>this gives:<br>warning: function 'foo' is not needed and will not be emitted [-Wunneeded-internal-declaration]<br><br>-Argyrios<br><br><blockquote type="cite"><br>On Fri, Oct 26, 2012 at 9:52 AM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br><blockquote type="cite">No its used right here in the same file. It only warns on C++11 enabled<br>builds.<br><br> virtual void getAnalysisUsage(AnalysisUsage &AU) const {<br> AU.setPreservesAll();<br> AU.addRequiredID(PreVerifyID);<br><br><br>On Fri, Oct 26, 2012 at 9:41 AM, Matthieu Monrocq<br><<a href="mailto:matthieu.monrocq@gmail.com">matthieu.monrocq@gmail.com</a>> wrote:<br><blockquote type="cite"><br><br><br>On Fri, Oct 26, 2012 at 8:51 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>wrote:<br><blockquote type="cite"><br>On Thu, Oct 25, 2012 at 10:48 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>><br>wrote:<br><blockquote type="cite">I think this change broke bootstrap builds with C++11 and -Werror<br>enabled.<br><br>lib/VMCore/Verifier.cpp:116:14: error: variable 'PreVerifyID' is not<br>needed<br>and will not be emitted [-Werror,-Wunneeded-internal-declaration]<br>static char &PreVerifyID = PreVerifier::ID;<br></blockquote><br>Yes, I think this change probably caused that. I'm unclear on what the<br>purpose of that warning is: it appears to be warning on variables<br>which are referenced but not odr-used, which seems like a pretty<br>questionable thing to warn on. It'd be easy enough to fix this by<br>teaching Sema::ShouldWarnIfUnusedFileScopedDecl to ignore references<br>(along with its existing check for const variables), but I'm not sure<br>that's the right fix, since I'm not really sure what the intent is<br>here.<br></blockquote><br><br><br>Is not PreVerifyID just an unused variable ? (not the difference with<br>PreVerifier::ID)<br><br>-- Matthieu<br></blockquote><br><br><br><br>--<br>~Craig<br></blockquote></blockquote><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></blockquote><br></blockquote></blockquote></div><br></body></html>