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
