<div class="gmail_quote">2010/10/26 Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div style="word-wrap:break-word"><div class="im"><br><div><div>On Oct 24, 2010, at 1:21 AM, Marcin Swiderski wrote:</div><br><blockquote type="cite"><span style="border-collapse:separate;font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium">Modified: cfe/trunk/lib/Analysis/CFG.cpp<br>
URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=117220&r1=117219&r2=117220&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=117220&r1=117219&r2=117220&view=diff</a><br>
==============================================================================<br>--- cfe/trunk/lib/Analysis/CFG.cpp (original)<br>+++ cfe/trunk/lib/Analysis/CFG.cpp Sun Oct 24 03:21:40 2010<br>@@ -912,15 +912,17 @@<br>      AppendStmt(Block, B, asc);<br>
    }<br><br>-    // If visiting RHS causes us to finish 'Block' and the LHS doesn't<br>-    // create a new block, then we should return RBlock.  Otherwise<br>-    // we'll incorrectly return NULL.<br>-    CFGBlock *RBlock = Visit(B->getRHS());<br>
-    CFGBlock *LBlock = Visit(B->getLHS(), AddStmtChoice::AsLValueNotAlwaysAdd);<br>-    return LBlock ? LBlock : RBlock;<br>+    Visit(B->getLHS(), AddStmtChoice::AsLValueNotAlwaysAdd);<br>+    return Visit(B->getRHS());<br>
  }</span></blockquote></div><br></div><div><div>Hi Marcin,</div><div><br></div><div>Shouldn't we invert the check that we had before?  Visit(B->getRHS()) isn't guaranteed to return a non-NULL block if visiting the LHS resulted in finishing up a CFGBlock.  That was the purpose of the previous logic (when the LHS was visited first).</div>
</div><div><br></div><font color="#888888"><div>Ted</div></font></div></blockquote><div><br></div><div>Because:</div><div>1. removing the check after swaping the order did not crash tests,</div><div>2. VisitChildren() is implemented this way,</div>
<div>I assumed that this is the right way to go. If I'm mistaken then shouldn't we add such check in other places with similar code e.g. VisitChildren()?</div></div>