r285289 - [Sema] -Wunused-variable warning for array variables should behave

[David Blaikie via cfe-commits]

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/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.

(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)


> +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/20161031/87848984/attachment-0001.html>