[cfe-commits] r166361 - in /cfe/trunk: lib/CodeGen/CGExpr.cpp lib/Sema/SemaExpr.cpp test/CodeGenCXX/const-init-cxx11.cpp test/CodeGenCXX/for-range.cpp test/CodeGenCXX/lambda-expressions.cpp test/SemaCXX/lambda-expressions.cpp
Argyrios Kyrtzidis
kyrtzidis at apple.com
Fri Oct 26 17:02:59 PDT 2012
On Oct 26, 2012, at 3:12 PM, Nico Weber <thakis at chromium.org> wrote:
> On Fri, Oct 26, 2012 at 2:19 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com> wrote:
>> On Oct 26, 2012, at 2:16 PM, Nico Weber <thakis at chromium.org> 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 <kyrtzidis at apple.com> wrote:
>>>> On Oct 26, 2012, at 1:35 PM, Richard Smith <richard at metafoo.co.uk> 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 <craig.topper at gmail.com> 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
>>>>>> <matthieu.monrocq at gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Fri, Oct 26, 2012 at 8:51 AM, Richard Smith <richard at metafoo.co.uk>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>> On Thu, Oct 25, 2012 at 10:48 PM, Craig Topper <craig.topper at gmail.com>
>>>>>>>> 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
>>>> 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/20121026/e39e9192/attachment.html>
More information about the cfe-commits
mailing list