[cfe-dev] RFC redundant assignment in switch

[Philip Reames]

On 03/31/2014 09:21 AM, Ted Kremenek wrote:
> On Mar 31, 2014, at 9:08 AM, Jordan Rose <jordan_rose at apple.com 
> <mailto:jordan_rose at apple.com>> wrote:
>
>>
>> On Mar 30, 2014, at 19:47 , Kyle Sluder <kyle at ksluder.com 
>> <mailto:kyle at ksluder.com>> wrote:
>>
>>> On Mar 28, 2014, at 1:52 PM, Jordan Rose <jordan_rose at apple.com 
>>> <mailto:jordan_rose at apple.com>> wrote:
>>>>
>>>> Hi, Daniel. We already have this in Clang itself as 
>>>> -Wimplicit-fallthrough,
>>>
>>> That warning triggers for every fallthrough. Daniel's checker only 
>>> triggers for assignments that are redundant with assignments made in 
>>> cases that are fallen into.
>>>
>>> These are philosophically different approaches, and they are 
>>> mutually compatible. Even if -Wimplicit-fallthrough becomes useful 
>>> to C/ObjC programmers via the __fallthrough macro, Daniel's checker 
>>> is still useful because it will catch logic bugs in code that 
>>> redundantly assigns to the same variable in two different cases.
>>
>> Ah, interesting. Isn't this already caught with the dead stores 
>> checker, though?
>>
>> *<stdin>:5:5: **warning: **Value stored to 'y' is never read*
>>     y = 1;
>> *    ^   ~*
>> *
>> *
>> We could probably stand to improve that checker to say 
>> /why/ something's a dead store, but I don't think we need a separate 
>> pass.
>>
>> Jordan
>
> Agreed.  I'd rather we have the general dead stores checker (and make 
> it better) then have something more specialized like this.  The dead 
> stores checker has also been hardened over several years to handle 
> many common idioms of defensive code that are "dead" but not interesting.
>
Speaking as someone who has tried to use the static analyser on a 
largish code base in the recent past, I would *much* rather see a 
separate checker.  We have literally hundreds of false positives from 
dead stores and actively ignore them.  A "dead store due to fall-through 
case" on the other hand is likely a bug and is thus very interesting.

To be clear, I'm most commenting on reporting *interface* not 
*implementation*.

Philip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140331/38d09451/attachment.html>