On Fri, Oct 26, 2012 at 2:19 PM, Argyrios Kyrtzidis <[email protected]> wrote: > On Oct 26, 2012, at 2:16 PM, Nico Weber <[email protected]> wrote: > >> FWIW, I found -Wunneeded-internal-declaration fairly confusing. I >> tried to turn it on for chromium, and it fired in a few instances >> where the function it warned about was used for something and couldn't >> be easily removed. > > Could you be more specific ? It's not clear if you are talking about a false > positive or something else.
I don't remember details. I did a new build with the flag right now, here's what it found: In this case, clang warns about _NullPool which is used in the function Null() which in turn is used in some translation units: 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/p/chromium/source/search?q=file%3Aharfbuzz+Null%5C%28&origq=file%3Aharfbuzz+Null%5C%28&btnG=Search+Trunk ../../third_party/harfbuzz-ng/src/hb-open-type-private.hh:136:20: error: variable '_NullPool' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] static const void *_NullPool[64 / sizeof (void *)]; ^ 1 error generated. I'm not sure what to do with this warning. A related example: clang thinks that GrInitialArrayAllocationCount() should be "static inline" (ok, I guess). However, GrNextArrayAllocationCount() right below it doesn't warn, even though the two functions seem to be used in the same way later in the file (in GrTDArray::growAt()). http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/skia/src/gpu/GrTDArray.h&exact_package=chromium&q=GrInitialArrayAllocationCount&l=17 In file included from ../../third_party/skia/src/gpu/gl/GrGpuGL.cpp:9: In file included from ../../third_party/skia/src/gpu/gl/GrGpuGL.h:23: In file included from ../../third_party/skia/src/gpu/gl/../GrTHashCache.h:14: ../../third_party/skia/src/gpu/GrTDArray.h:17:12: error: 'static' function 'GrInitialArrayAllocationCount' declared in header file should be declared 'static inline' [-Werror,-Wunneeded-internal-declaration] static int GrInitialArrayAllocationCount() { ^ 1 error generated. …and that's all already. The last time I tried I think there were more issues. Nico > > -Argyrios > >> >> Nico >> >> On Fri, Oct 26, 2012 at 1:54 PM, Argyrios Kyrtzidis <[email protected]> >> wrote: >>> On Oct 26, 2012, at 1:35 PM, Richard Smith <[email protected]> wrote: >>> >>>> Argyrios: It looks like you added this warning in r129794. Can you >>>> comment on what it's intended to detect? >>> >>> variables/functions with internal linkage that are not used from the >>> codegen perspective. >>> This differs from -Wunused which will consider a 'use' even in an >>> unevaluated context. >>> For example: >>> >>> static void foo() { } >>> >>> this gives: >>> warning: unused function 'foo' [-Wunused-function] >>> >>> static void foo() { } >>> template <typename T> >>> void goo() { >>> foo(); >>> } >>> >>> this gives: >>> warning: function 'foo' is not needed and will not be emitted >>> [-Wunneeded-internal-declaration] >>> >>> -Argyrios >>> >>>> >>>> On Fri, Oct 26, 2012 at 9:52 AM, Craig Topper <[email protected]> >>>> wrote: >>>>> No its used right here in the same file. It only warns on C++11 enabled >>>>> builds. >>>>> >>>>> virtual void getAnalysisUsage(AnalysisUsage &AU) const { >>>>> AU.setPreservesAll(); >>>>> AU.addRequiredID(PreVerifyID); >>>>> >>>>> >>>>> On Fri, Oct 26, 2012 at 9:41 AM, Matthieu Monrocq >>>>> <[email protected]> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Oct 26, 2012 at 8:51 AM, Richard Smith <[email protected]> >>>>>> wrote: >>>>>>> >>>>>>> On Thu, Oct 25, 2012 at 10:48 PM, Craig Topper <[email protected]> >>>>>>> wrote: >>>>>>>> I think this change broke bootstrap builds with C++11 and -Werror >>>>>>>> enabled. >>>>>>>> >>>>>>>> lib/VMCore/Verifier.cpp:116:14: error: variable 'PreVerifyID' is not >>>>>>>> needed >>>>>>>> and will not be emitted [-Werror,-Wunneeded-internal-declaration] >>>>>>>> static char &PreVerifyID = PreVerifier::ID; >>>>>>> >>>>>>> Yes, I think this change probably caused that. I'm unclear on what the >>>>>>> purpose of that warning is: it appears to be warning on variables >>>>>>> which are referenced but not odr-used, which seems like a pretty >>>>>>> questionable thing to warn on. It'd be easy enough to fix this by >>>>>>> teaching Sema::ShouldWarnIfUnusedFileScopedDecl to ignore references >>>>>>> (along with its existing check for const variables), but I'm not sure >>>>>>> that's the right fix, since I'm not really sure what the intent is >>>>>>> here. >>>>>> >>>>>> >>>>>> >>>>>> Is not PreVerifyID just an unused variable ? (not the difference with >>>>>> PreVerifier::ID) >>>>>> >>>>>> -- Matthieu >>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> ~Craig >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
