On Oct 26, 2012, at 3:12 PM, Nico Weber <[email protected]> wrote:
> 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. It says so in the comment above it :-) > /* TODO This really should be a extern HB_INTERNAL and defined somewhere... */ We could have the warning suppressed for static variables in headers though. > > > > 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. 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. -Argyrios > > > > …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
