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

[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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120117/364e47e0/attachment.html>