[cfe-commits] r166625 - in /cfe/trunk: lib/Sema/SemaDecl.cpp test/SemaCXX/warn-unused-variables.cpp

[David Blaikie]

On Thu, Oct 25, 2012 at 10:05 AM, Matthieu Monrocq
<matthieu.monrocq at gmail.com> wrote:
>
>
> On Wed, Oct 24, 2012 at 11:29 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> Author: dblaikie
>> Date: Wed Oct 24 16:29:06 2012
>> New Revision: 166625
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=166625&view=rev
>> Log:
>> Fix false positive in -Wunused-variable when a ctor call make involve
>> cleanups.
>>
>> Modified:
>>     cfe/trunk/lib/Sema/SemaDecl.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=166625&r1=166624&r2=166625&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Oct 24 16:29:06 2012
>> @@ -1286,6 +1286,8 @@
>>            return false;
>>
>>          if (const Expr *Init = VD->getInit()) {
>> +          if (const ExprWithCleanups *Cleanups =
>> dyn_cast<ExprWithCleanups>(Init))
>> +            Init = Cleanups->getSubExpr();
>>            const CXXConstructExpr *Construct =
>>              dyn_cast<CXXConstructExpr>(Init);
>>            if (Construct && !Construct->isElidable()) {
>>
>> 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=166625&r1=166624&r2=166625&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Wed Oct 24 16:29:06
>> 2012
>> @@ -123,3 +123,15 @@
>>      S3 z = a; // expected-warning {{unused variable 'z'}}
>>    }
>>  }
>> +
>> +namespace ctor_with_cleanups {
>> +  struct S1 {
>> +    ~S1();
>> +  };
>> +  struct S2 {
>> +    S2(const S1&);
>> +  };
>> +  void func() {
>> +    S2 s((S1()));
>> +  }
>> +}
>>
>
> So, here S1() will generate a cleanup... however "s" is still unused and of
> little interest because if I were to write "S1();" there would be cleanup
> and no "s", isn't it ?

Well S2(const S1&) might actually do work (since we can't see its
implementation), so you couldn't just write "S1()", you'd need to
write "S2(S1())". But you're right, you wouldn't need to name the
variable.

> I don't see the point in introducing a named variable when the effect of not
> introducing it would be identical.

I agree to an extent. But this was an existing decision in PR11550
that just had a bug in it.

I brought this up in the cfe-dev thread "PR9824: -Wunused-but-set"
which you might like to read the responses to & reply there if you
have further thoughts/opinions on the semantics here.