On 31 October 2016 at 15:34, David Blaikie <dblai...@gmail.com> wrote:
> > > On Thu, Oct 27, 2016 at 6:40 AM Alex Lorenz via cfe-commits < > cfe-commits@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/ >> SemaDecl.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 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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
