<bump>

On Wed, Dec 14, 2011 at 12:12 PM, David Blaikie <[email protected]> wrote:
> On Sat, Sep 24, 2011 at 7:21 AM, Ted Kremenek <[email protected]> wrote:
>> On Sep 23, 2011, at 9:33 PM, David Blaikie wrote:
>>
>>> Hi Ted,
>>>
>>> I tried -Wunreachable-code earlier today (Chandler had suggested it as a 
>>> way to find/remove the dead code after llvm_unreachables I'd migrated 
>>> yesterday) & it produced some very... interesting output. It did find the 
>>> dead code after llvm_unreachable but it also found some other very strange 
>>> cases. I was wondering what was up with that. Good to know it's WIP - any 
>>> tips on the state of that? anywhere I'd be able to lend a hand?
>>
>> Hi David,
>>
>> The weirdest results I see from -Wunreachable-code tend to involve template 
>> code.  In templates, I've seen cases where a branch condition depends on a 
>> template parameter, and at template instantiation the branch condition may 
>> become a constant.  This can cause some code to (correctly) be flagged as 
>> unreachable for that particular instantiation of a template.  This of course 
>> is not an invariant for that template for *all* instantiations, so it's not 
>> a real issue.
>>
>> The solution I had in mind to fix this problem is to enhance the CFG.  
>> Instead of just pruning CFG edges, for templates we could record when an 
>> edge was pruned AND whether or not it was dependent on a template parameter. 
>>  Most analyses would  continue to see the CFG as they do now, but specific 
>> analyses (such as -Wunreachable-code) could do something a bit more clever 
>> and not treat such code as unreachable.
>>
>>>
>>> It wouldn't catch all the same cases ("case N: default: stuff" for example) 
>>> but this solution isn't great either, it'll catch a variety of arcane cases 
>>> that won't have trivial fixes.
>>
>> Ah, interesting.  -Wunreachable-code looks for finding unreachable basic 
>> blocks, not looking at whether or not a label could never be used.  Those 
>> are different concepts.
>>
>>> Chandler had mentioned the idea of this warning (well, something like it) 
>>> yesterday but after I threw this together we were talking about it more & 
>>> realized it'd be pretty tricky to get right with a nice multiline fixit 
>>> that is very reliable (I get the impression that's what he's really 
>>> interested in - really low (0?) false positive rate & accurate fixits - 
>>> which would be awesome, but require a rather different fix)
>>
>> I agree.  A warning like this in the compiler needs close to a zero false 
>> positive rate.
>
> I'm just coming back to this (checking all my "loose ends" threads, or
> at least some of them) and I can't quite remember what the problem was
> here. Is there a case my warning would fire on that shouldn't be
> fixed? I don't think so. The real issue would be the fixit - removing
> the default label would always be valid, but that could introduce
> (probably fairly trivial) unreachable code. Adding a fixit to remove
> the whole block would be harder. Should we have the warning with no
> fixit at all?
>
> Also, I'm still interested in checking in the mechanical fixes & so
> far as I know it's the intended/preferred way of writing switches in
> Clang - it's usually given as code review feedback, but as with
> everything that isn't verified, things slip through. So if someone
> wants to confirm/sign off on that I'll check those in & we can nut out
> the warning separately.
>
> - David

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to