On 1 November 2016 at 10:13, Alex L <arpha...@gmail.com> wrote: > > 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/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@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