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

[Richard Smith]

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

> On Mon, Jul 16, 2012 at 5:40 PM, Chandler Carruth <chandlerc at google.com>wrote:
>
>> 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.
>>
>
> That doesn't fall naturally out of the algorithm, so I'd be concerned that
> the added complexity doesn't justify the benefit.
>
> I'm coming round to Jordy's position. In particular, for "int n; n *= 0;
> return n;", warning on the return statement does not seem particularly
> reasonable, and warning on the *= is pedantically correct, since it has
> undefined behavior. The code samples in which I found the 'false' positives
> are fairly questionable.
>

OK, r160334 causes us to diagnose these cases.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120716/2e583cc8/attachment.html>