[cfe-commits] r160330 - in /cfe/trunk: lib/Analysis/UninitializedValues.cpp test/Sema/uninit-variables.c test/SemaCXX/uninit-variables.cpp

[Chandler Carruth]

On Mon, Jul 16, 2012 at 5:27 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Mon, Jul 16, 2012 at 5:15 PM, Jordan Rose <jordan_rose at apple.com>wrote:
>
>>
>> On Jul 16, 2012, at 17:06 , Richard Smith <richard-llvm at metafoo.co.uk>
>> wrote:
>>
>> int test6() {
>>   int x; // expected-note{{initialize the variable 'x' to silence this
>> warning}}
>> -  x += 2; // expected-warning{{variable 'x' is uninitialized when used
>> here}}
>> -  return x;
>> +  x += 2;
>> +  return x; // expected-warning{{variable 'x' is uninitialized when used
>> here}}
>> }
>>
>>
>> Sorry for not chiming in pre-commit, but I'm not happy with this. The
>> first non-compound-assignment use of a variable could be very far from the
>> variable, and while we do get a note on the decl, it still seems like a QoI
>> problem. I'd rather have compound assignments treated as a use *and* an
>> initialization.
>>
>> (Actually, can we just call it a use? Does that hurt anything?)
>>
>
> I mentioned this on the review thread: it introduced "false" positives
> into my test corpus (FSVO "false"). Basically, if a variable is only ever
> used in compound-assignments, but its value is never actually used, we have
> not proven that there is a "real" problem with the code (FSVO "real").
>
> I don't see how the distance between the variable and the use which we
> report is a problem, since we give diagnostics pointing at both of them. I
> would prefer to not add this extra checking (which seems to basically be
> all noise) into -Wuninitialized, but if you feel strongly about this, I'll
> make the change.
>

While using your condition for triggering the warning at all, could we
attach the warning to the 'x += 2', and note the final use? Or at least
note the 'x += 2'? That would seem to address most of Jordan's QoI concern
w/o increasing false positives.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120716/8921ebaf/attachment.html>