<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thank you!<div class=""><br class=""><div style=""><blockquote type="cite" class=""><div class="">On Aug 7, 2014, at 11:54 AM, Manuel Klimek <<a href="mailto:klimek@google.com" class="">klimek@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Ok, fixed this in r215128 ... That indeed makes the code significantly simpler.</div><div class="gmail_extra"><br class=""><br class=""><div class="gmail_quote">On Thu, Aug 7, 2014 at 7:15 PM, Manuel Klimek <span dir="ltr" class=""><<a href="mailto:klimek@google.com" target="_blank" class="">klimek@google.com</a>></span> wrote:<br class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div class=""><div class="h5"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 7, 2014 at 7:14 PM, Ted Kremenek <span dir="ltr" class=""><<a href="mailto:kremenek@apple.com" target="_blank" class="">kremenek@apple.com</a>></span> wrote:<br class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class=""><div class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Aug 7, 2014, at 10:10 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="">klimek@google.com</a>> wrote:</div>
<br class=""><div class=""><span style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">On Thu, Aug 7, 2014 at 7:06 PM, Ted Kremenek<span class=""> </span></span><span dir="ltr" style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class=""><<a href="mailto:kremenek@apple.com" target="_blank" class="">kremenek@apple.com</a>></span><span style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class=""> </span><span style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important" class="">wrote:</span><br style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">
<blockquote class="gmail_quote" style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Hi Manuel,<br class=""><br class="">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:<br class="">
<br class=""> <span class=""> </span>if (sizeof(foo) > 4) { /* possibly dead; -Wunreachable-code doesn't warn */ )<br class=""><br class="">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:<br class="">
<br class=""> <span class=""> </span>addSuccessor(Block, ThenBlock, /* isReachable = */ !KnownVal.isFalse());<br class=""> <span class=""> </span>addSuccessor(Block, ElseBlock, /* isReachable = */ !KnownVal.isTrue());<br class=""><br class="">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?<br class="">
</blockquote><div style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">
<br class=""></div><div style="font-family:Helvetica;font-size:14px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px" class="">
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...</div></div></blockquote></div>
<div class="">
<br class=""></div></div>Thanks Manuel.<div class=""><br class=""><div class="">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'.</div>
<div class=""><br class=""></div><div class="">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.</div>
</div></div></blockquote></div><br class=""></div></div></div><div class="gmail_extra">I actually think I have added all the tests already, and just have to flip them to not warn :)</div></div>
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>