<div class="gmail_quote">On Wed, Jan 18, 2012 at 3:04 AM, Erik Verbruggen <span dir="ltr"><<a href="mailto:erik.verbruggen@me.com">erik.verbruggen@me.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On 17-1-12 17:37, Ted Kremenek wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Tuesday, January 17, 2012 at 8:22 AM, Erik Verbruggen wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On 17-1-12 7:02, Chandler Carruth wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On Mon, Jan 16, 2012 at 9:29 PM, Ted Kremenek <<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a><br>
<mailto:<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>><br>
<mailto:<a href="mailto:kremenek@apple.com" target="_blank">kremenek@apple.com</a>>> wrote:<br>
<br>
Hi Erik,<br>
<br>
Thanks for working on this. I've commented in the PR.<br>
<br>
My concern is that I am having second thoughts about whether or not<br>
this is the right direction. If there are temporaries in the return<br>
expression, should those destructors appear before the 'return'<br>
statement (but after the subexpression of 'return' gets evaluated)?<br>
I can see it both ways.<br>
<br>
For example:<br>
<br>
return A();<br>
<br>
where 'A' converts to whatever the return value type should be, and<br>
~A() is non-trivial. Technically, ~A() evaluates after the<br>
enclosing statement. We're giving 'return' special treatment in<br>
your patch.<br>
<br>
One clean representation of this is to put all the destructors after<br>
the 'return' in the CFGBlock. The other way is to have the<br>
destructors appear after the subexpression of 'return', but before<br>
the 'return' itself. The former requires clients of the CFG to<br>
rethink where they expect 'return' to be in a CFGBlock.<br>
<br>
What do you think?<br>
<br>
<br>
If I can jump in from the peanut gallery...<br>
<br>
This doesn't seem a necessarily unique problem to return statements. Any<br>
statement which forms a CFGBlock terminator and which contains<br>
temporaries with destructors should hit the same issue. The other<br>
example is I suspect throw. It may even be possible to trigger this with<br>
an indirect goto if the index into the label-address array involves<br>
temporary objects.<br>
</blockquote>
<br>
For a throw it is actually slightly worse: no implicit destructors calls<br>
get added to the CFG. I did not check the indirect goto, because I could<br>
not come up with a good example..<br>
<br>
</blockquote>
For now, just assume that everything with EH in the CFG is wrong or<br>
incomplete. That's a discussion on its own.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I think the C++ standard* provides some help in making a choice about<br>
how to represent this. It doesn't actually say that ~A() evaluates after<br>
the enclosing statement. Instead it defines 'full-expressions' as an<br>
expression which is not a subexpression of any other expression<br>
([intro.execution] 1.9p10). As *part* of this full-expression, all<br>
temporary objects' non-trivial destructors must be called<br>
([class.temporary] 12.2p3). To me, that indicates that the destructor<br>
should be placed after the complete subexpression in the return<br>
statement, but within the return statement itself. This has the nice<br>
property of preserving the (very useful!) invariant that terminator<br>
statements are the last statement in the CFG block.<br>
</blockquote>
<br>
This would mean that a full expression of "return SomeClass();" would<br>
get split in two: all the implicit destructors calls for the scope get<br>
placed between the return value calculation and the actual return of<br>
that value.<br>
</blockquote>
Yes. Exactly. That's the correct semantics<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
If you would want to base code generation on the CFG, it<br>
would force an extra pass to identify whether calculated values should<br>
be constructed in a return value slot or not. So I think that using<br>
return/throw (and possibly goto) as markers, would be a simplification.<br>
</blockquote>
Concerning "goto" and "throw"; those are terminators and already can<br>
serve as markers. I'll elaborate with the case of "return". I'm also not<br>
certain what you mean by an "extra pass", but it *might* require some work.<br>
<br>
My concern is that, unlike "goto", the "return" in the CFG is not a<br>
terminator; it doesn't represent a transfer of control. That's what<br>
terminators in CFGBlocks are for. Instead, it appears in the CFGBlock as<br>
an element, which means (in that context) it represents a statement that<br>
gets executed. Since it doesn't represent the transfer of control in<br>
that context, the sole role of the "return" in the CFG is to represent<br>
the binding of the return value.<br>
<br>
That said, we could place the "return" as a terminator, in *addition* to<br>
what we do now, and have that terminator terminate the CFGBlock with the<br>
destructor calls in it. That terminator would be on a CFGBlock that<br>
transitioned to the Exit block. Doing this isn't absolutely necessary,<br>
but it would annotate the end of the CFGBlock with the return statement,<br>
possibly making it more amendable to code generation while still<br>
preserving the invariants of the CFG.<br>
</blockquote>
<br></div></div>
So if I understand it correctly, the following example should generate the CFG below:<div class="im"><br>
<br>
class A {<br>
public:<br>
A();<br>
~A();<br>
};<br>
<br></div>
class B {<br>
public:<br>
B();<br>
~B();<br>
};<br>
<br>
B test() {<br>
A a;<br>
return B();<div class="im"><br>
}<br>
<br>
[B3 (ENTRY)]<br>
Succs (1): B2<br>
<br>
[B1]<br>
1: [B2.2].~A() (Implicit destructor)<br>
Succs (1): B0<br>
<br>
[B2]<br>
1: (CXXConstructExpr, class A)<br>
2: A a;<br></div>
3: B() (CXXConstructExpr, class B)<br>
4: [B2.3] (BindTemporary)<br>
5: [B2.4] (ImplicitCastExpr, NoOp, const class B)<br>
6: [B2.5]<br>
7: [B2.6] (CXXConstructExpr, class B)<br>
8: ~B() (Temporary object destructor)<br>
9: return [B2.7];<br>
Preds (1): B3<br>
Succs (1): B1<div class="im"><br>
<br>
[B0 (EXIT)]<br>
Preds (2): B1<br>
<br></div>
(Note: this is manually "corrected" CFG.)<br>
<br>
Is that right?</blockquote><div><br></div><div>FWIW, this what I had in mind. I'll also give a slightly more complex example to further illustrate my idea, but again this is a manually "corrected" CFG:</div>
<div><br></div><div><div class="im">B test2() {</div> A a1;</div><div> {</div><div> A a2, a3;</div><div> A a4;</div><div> return B();</div><div><div class="im"> }<br>}<br><br> [B5 (ENTRY)]<br> Succs (1): B4<br>
<div class="im"><br></div><div class="im"> [B4]<br> 1: (CXXConstructExpr, class A)<br> 2: A a1,</div><div> Preds (1): B5<br></div><div> Succs (1): B4<br></div><div><br></div> [B3]<br> 1: (CXXConstructExpr, class A)<br>
2: A a2,</div><div class="im"> 3: (CXXConstructExpr, class A)<br> 4: a3;</div><div class="im"> 5: (CXXConstructExpr, class A)<br> 6: A a4;</div> 7: B() (CXXConstructExpr, class B)<br> 8: [B3.7] (BindTemporary)<br>
9: [B3.8] (ImplicitCastExpr, NoOp, const class B)<br> 10: [B3.9]<br> 11: [B3.10] (CXXConstructExpr, class B)<br> 12: ~B() (Temporary object destructor)<br> 13: return [B3.11];<br> Preds (1): B4<br> Succs (1): B1<div class="im">
<br> [B2]<br></div><div class="im"> 1: [B3.6].~A() (Implicit destructor)</div><div class="im"> 2: [B3.4].~A() (Implicit destructor)</div><div class="im"> 3: [B3.2].~A() (Implicit destructor)</div><div class="im"> Preds (1): B3<br>
Succs (1): B1<br><br> [B1]<br> 1: [B4.2].~A() (Implicit destructor)</div><div class="im"> Preds (1): B2<br> Succs (1): B0<br><br> [B0 (EXIT)]<br> Preds (1): B1<br></div></div></div>