r285289 - [Sema] -Wunused-variable warning for array variables should behave
Alex L via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 1 03:14:49 PDT 2016
On 1 November 2016 at 10:13, Alex L <arphaman at gmail.com> wrote:
>
> On 31 October 2016 at 15:34, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Thu, Oct 27, 2016 at 6:40 AM Alex Lorenz via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: arphaman
>>> Date: Thu Oct 27 08:30:51 2016
>>> New Revision: 285289
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=285289&view=rev
>>> Log:
>>> [Sema] -Wunused-variable warning for array variables should behave
>>> similarly to scalar variables.
>>>
>>> This commit makes the -Wunused-variable warning behaviour more
>>> consistent:
>>> Now clang won't warn for array variables where it doesn't warn for scalar
>>> variables.
>>>
>>> rdar://24158862
>>>
>>> Differential Revision: https://reviews.llvm.org/D25937
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaDecl.cpp
>>> cfe/trunk/test/SemaCXX/warn-everthing.cpp
>>> cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaD
>>> ecl.cpp?rev=285289&r1=285288&r2=285289&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Oct 27 08:30:51 2016
>>> @@ -1522,7 +1522,7 @@ static bool ShouldDiagnoseUnusedDecl(con
>>> if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
>>>
>>> // White-list anything with an __attribute__((unused)) type.
>>> - QualType Ty = VD->getType();
>>> + const auto *Ty = VD->getType().getTypePtr();
>>>
>>> // Only look at the outermost level of typedef.
>>> if (const TypedefType *TT = Ty->getAs<TypedefType>()) {
>>> @@ -1535,6 +1535,10 @@ static bool ShouldDiagnoseUnusedDecl(con
>>> if (Ty->isIncompleteType() || Ty->isDependentType())
>>> return false;
>>>
>>> + // Look at the element type to ensure that the warning behaviour is
>>> + // consistent for both scalars and arrays.
>>> + Ty = Ty->getBaseElementTypeUnsafe();
>>> +
>>> if (const TagType *TT = Ty->getAs<TagType>()) {
>>> const TagDecl *Tag = TT->getDecl();
>>> if (Tag->hasAttr<UnusedAttr>())
>>>
>>> Modified: cfe/trunk/test/SemaCXX/warn-everthing.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/
>>> warn-everthing.cpp?rev=285289&r1=285288&r2=285289&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/SemaCXX/warn-everthing.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/warn-everthing.cpp Thu Oct 27 08:30:51 2016
>>> @@ -9,5 +9,5 @@ public:
>>> };
>>>
>>> void testPR12271() { // expected-warning {{no previous prototype for
>>> function 'testPR12271'}}
>>> - PR12271 a[1][1]; // expected-warning {{unused variable 'a'}}
>>> + PR12271 a[1][1];
>>> }
>>>
>>> Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/
>>> warn-unused-variables.cpp?rev=285289&r1=285288&r2=285289&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Thu Oct 27
>>> 08:30:51 2016
>>> @@ -150,3 +150,54 @@ namespace ctor_with_cleanups {
>>> }
>>>
>>> #include "Inputs/warn-unused-variables.h"
>>> +
>>> +namespace arrayRecords {
>>> +
>>> +int total = 0;
>>> +
>>> +class Adder {
>>>
>>
>> Presumably this class could be a bit simpler - is it just about having a
>> non-trivial ctor? or non-trivial dtor?
>>
>> It'd be helpful to make the test as simple as possible to show what
>> features are important to the diagnostic - rather than making the test look
>> like real code by having functionality that's not required for testing.
>>
>> (eg: you don't have to implement functions here - the code will not be
>> linked or executed, just compiled for warnings in the test suite)
>>
>> Potentially name the class by its purpose:
>>
>> struct NonTriviallyDestructible {
>> ~NonTriviallyDestructible();
>> };
>>
>> and similar.
>>
>
> Thanks for looking at this commit!
> You're right about this particular class, it can be simpler, since it's
> testing a non-trivial door. When I started working on this patch I used the
>
s/door/dtor/.
> code from the bug report to reproduce this issue in the test case, and
> didn't simplify it further when I found out the cause.
>
>
>>
>> (is it important that Adder and Foo are constructed with arguments/have
>> ctor parameters? It's not clear to me from the test or code whether that's
>> the case)
>>
>
> No. Do you think I should simplify this test case in a separate commit?
>
> Cheers,
> Alex
>
>
>>
>>
>>> +public:
>>> + Adder(int x); // out of line below
>>> + ~Adder() {}
>>> +};
>>> +
>>> +Adder::Adder(int x) {
>>> + total += x;
>>> +}
>>> +
>>> +struct Foo {
>>> + int x;
>>> + Foo(int x) : x(x) {}
>>> +};
>>> +
>>> +struct S1 {
>>> + S1();
>>> +};
>>> +
>>> +void foo(int size) {
>>> + S1 y; // no warning
>>> + S1 yarray[2]; // no warning
>>> + S1 dynArray[size]; // no warning
>>> + S1 nestedArray[1][2][3]; // no warning
>>> +
>>> + Adder scalerInFuncScope = 134; // no warning
>>> + Adder arrayInFuncScope[] = { 135, 136 }; // no warning
>>> + Adder nestedArrayInFuncScope[2][2] = { {1,2}, {3,4} }; // no warning
>>> +
>>> + Foo fooScalar = 1; // expected-warning {{unused variable 'fooScalar'}}
>>> + Foo fooArray[] = {1,2}; // expected-warning {{unused variable
>>> 'fooArray'}}
>>> + Foo fooNested[2][2] = { {1,2}, {3,4} }; // expected-warning {{unused
>>> variable 'fooNested'}}
>>> +}
>>> +
>>> +template<int N>
>>> +void bar() {
>>> + Adder scaler = 123; // no warning
>>> + Adder array[N] = {1,2}; // no warning
>>> +}
>>> +
>>> +void test() {
>>> + foo(10);
>>> + bar<2>();
>>> +}
>>> +
>>> +}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161101/6c6f00c2/attachment-0001.html>
More information about the cfe-commits
mailing list