On Thu, Aug 7, 2014 at 7:14 PM, Ted Kremenek <[email protected]> wrote:
> > On Aug 7, 2014, at 10:10 AM, Manuel Klimek <[email protected]> wrote: > > On Thu, Aug 7, 2014 at 7:06 PM, Ted Kremenek <[email protected]> wrote: > >> Hi Manuel, >> >> This is a good optimization, but I worry about its implications on >> -Wunreachable-code. We have a whole bunch of heuristics in >> -Wunreachable-code to not warn about dead code that is really only >> conditionally dead because of platform-specific constants. For example: >> >> if (sizeof(foo) > 4) { /* possibly dead; -Wunreachable-code doesn't >> warn */ ) >> >> To model this in the CFG, we actually still add the successors, but mark >> them as being possibly unreachable. For example, here is some code >> elsewhere in CFG.cpp: >> >> addSuccessor(Block, ThenBlock, /* isReachable = */ >> !KnownVal.isFalse()); >> addSuccessor(Block, ElseBlock, /* isReachable = */ !KnownVal.isTrue()); >> >> What happens is that most clients of the CFG see that the branch is dead, >> but other clients can recover the original edge. By just outright pruning >> the edge we lose this information. We want the same heuristics for dead >> code warnings to apply to ternary operators as well. Instead of doing an >> early return, can you just use this modified version of addSuccessor? >> > > Ok, I have a current fix for a more obvious bug and will add tests for > platform specific constants next. Otherwise also feel free to revert in the > meantime if it breaks something for you... > > > Thanks Manuel. > > I don't think it is breaking anything; it will just introduce false > positives for -Wunreachable-code that ideally we will want to plug soon. I > think the natural tests here would be to extend the -Wunreachable-code test > cases with more examples of ternary operations, and how we don't warn on > cases that truly rely on platform specific constants. We already have > examples of this already for 'if'. > > The upshot is that we want the CFG to be able to retain some of the > information for unreachable blocks as some analyses don't want to see the > pruning. It's a fundamental design shift we introduced in the analyzer 4-5 > months ago. Before we just hard pruned branches. > I actually think I have added all the tests already, and just have to flip them to not warn :)
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
