Thank you! > On Aug 7, 2014, at 11:54 AM, Manuel Klimek <[email protected]> wrote: > > Ok, fixed this in r215128 ... That indeed makes the code significantly > simpler. > > > On Thu, Aug 7, 2014 at 7:15 PM, Manuel Klimek <[email protected] > <mailto:[email protected]>> wrote: > On Thu, Aug 7, 2014 at 7:14 PM, Ted Kremenek <[email protected] > <mailto:[email protected]>> wrote: > >> On Aug 7, 2014, at 10:10 AM, Manuel Klimek <[email protected] >> <mailto:[email protected]>> wrote: >> >> On Thu, Aug 7, 2014 at 7:06 PM, Ted Kremenek <[email protected] >> <mailto:[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
