<br><br><div class="gmail_quote">2010/10/21 Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>></span><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<div><div></div><div class="h5"><br>
On Oct 20, 2010, at 9:29 PM, Ted Kremenek wrote:<br>
<br>
><br>
> On Oct 17, 2010, at 3:06 PM, Marcin Świderski wrote:<br>
><br>
>> I'm currently working on modeling destructors of temporary objects in CFG. I've attached patch with prototype implementation and some tests.<br>
>><br>
><br>
> Hi Marcin,<br>
><br>
> Aside from your other comment (which I will get to later), I had a question about the following:<br>
><br>
> +CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E) {<br>
> +  if (E->isLogicalOp()) {<br>
> +    // Destructors for temporaries in LHS expression should be called after<br>
> +    // those for RHS expression. Even if this will unnecessarily create a block,<br>
> +    // this block will be used at least by the full expression.<br>
><br>
> In the CFG, where we must represent control-flow between the LHS and RHS subexpressions, we actually have the RHS appear first in the CFG.  I believe, however, that the compiler chooses the reverse direction.  Should we reverse it to match?  I think your logic here assumes that the LHS temporaries are created first.<br>

><br>
> Ted<br>
<br>
</div></div>More specifically, my comment applies to the following code:<br>
<div class="im"><br>
+CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E) {<br>
+  if (E->isLogicalOp()) {<br>
+    // Destructors for temporaries in LHS expression should be called after<br>
+    // those for RHS expression. Even if this will unnecessarily create a block,<br>
+    // this block will be used at least by the full expression.<br>
<br>
</div>Basically this assumption doesn't match how we treat binary operators in the CFG.  We should probably fix having the LHS always precessed first.  Any objections?</blockquote><div><br>I agree. So  the fix would be in CFGBlock *CFGBuilder::VisitChildren(Stmt* Terminator), visit the children in reverse order.<br>
</div></div><br>