[cfe-commits] [Review] Partial fix for PR 11645: put destructor calls before a return statement.

[Xu Zhongxing]

? 2012/1/18 0:37, Ted Kremenek ??:
> On Tuesday, January 17, 2012 at 8:22 AM, Erik Verbruggen wrote:
>> On 17-1-12 7:02, Chandler Carruth wrote:
>>> On Mon, Jan 16, 2012 at 9:29 PM, Ted Kremenek <kremenek at apple.com 
>>> <mailto:kremenek at apple.com>
>>> <mailto:kremenek at apple.com>> wrote:
>>>
>>> Hi Erik,
>>>
>>> Thanks for working on this. I've commented in the PR.
>>>
>>> My concern is that I am having second thoughts about whether or not
>>> this is the right direction. If there are temporaries in the return
>>> expression, should those destructors appear before the 'return'
>>> statement (but after the subexpression of 'return' gets evaluated)?
>>> I can see it both ways.
>>>
>>> For example:
>>>
>>> return A();
>>>
>>> where 'A' converts to whatever the return value type should be, and
>>> ~A() is non-trivial. Technically, ~A() evaluates after the
>>> enclosing statement. We're giving 'return' special treatment in
>>> your patch.
>>>
>>> One clean representation of this is to put all the destructors after
>>> the 'return' in the CFGBlock. The other way is to have the
>>> destructors appear after the subexpression of 'return', but before
>>> the 'return' itself. The former requires clients of the CFG to
>>> rethink where they expect 'return' to be in a CFGBlock.
>>>
>>> What do you think?
>>>
>>>
>>> If I can jump in from the peanut gallery...
>>>
>>> This doesn't seem a necessarily unique problem to return statements. Any
>>> statement which forms a CFGBlock terminator and which contains
>>> temporaries with destructors should hit the same issue. The other
>>> example is I suspect throw. It may even be possible to trigger this with
>>> an indirect goto if the index into the label-address array involves
>>> temporary objects.
>>
>> For a throw it is actually slightly worse: no implicit destructors calls
>> get added to the CFG. I did not check the indirect goto, because I could
>> not come up with a good example..
>>
> For now, just assume that everything with EH in the CFG is wrong or 
> incomplete.  That's a discussion on its own.
>>> I think the C++ standard* provides some help in making a choice about
>>> how to represent this. It doesn't actually say that ~A() evaluates after
>>> the enclosing statement. Instead it defines 'full-expressions' as an
>>> expression which is not a subexpression of any other expression
>>> ([intro.execution] 1.9p10). As *part* of this full-expression, all
>>> temporary objects' non-trivial destructors must be called
>>> ([class.temporary] 12.2p3). To me, that indicates that the destructor
>>> should be placed after the complete subexpression in the return
>>> statement, but within the return statement itself. This has the nice
>>> property of preserving the (very useful!) invariant that terminator
>>> statements are the last statement in the CFG block.
>>
>> This would mean that a full expression of "return SomeClass();" would
>> get split in two: all the implicit destructors calls for the scope get
>> placed between the return value calculation and the actual return of
>> that value.
> Yes.  Exactly.  That's the correct semantics
>> If you would want to base code generation on the CFG, it
>> would force an extra pass to identify whether calculated values should
>> be constructed in a return value slot or not. So I think that using
>> return/throw (and possibly goto) as markers, would be a simplification.
> Concerning "goto" and "throw"; those are terminators and already can 
> serve as markers.  I'll elaborate with the case of "return".  I'm also 
> not certain what you mean by an "extra pass", but it *might* require 
> some work.
>
> My concern is that, unlike "goto", the "return" in the CFG is not a 
> terminator; it doesn't represent a transfer of control.  That's what 
> terminators in CFGBlocks are for.  Instead, it appears in the CFGBlock 
> as an element, which means (in that context) it represents a statement 
> that gets executed.  Since it doesn't represent the transfer of 
> control in that context, the sole role of the "return" in the CFG is 
> to represent the binding of the return value.
>
> That said, we could place the "return" as a terminator, in *addition* 
> to what we do now, and have that terminator terminate the CFGBlock 
> with the destructor calls in it. That terminator would be on a 
> CFGBlock that transitioned to the Exit block.  Doing this isn't 
> absolutely necessary, but it would annotate the end of the CFGBlock 
> with the return statement, possibly making it more amendable to code 
> generation while still preserving the invariants of the CFG.

What is the real benefits of having a ReturnStmt appeared as a 
terminator in addition to as a regular element? Not all functions have 
return statements.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120118/f83f692f/attachment.html>