[cfe-dev] CFG temporary objects destructors

[Zhongxing Xu]

2010/10/21 Ted Kremenek <kremenek at apple.com>

>
> On Oct 20, 2010, at 9:29 PM, Ted Kremenek wrote:
>
> >
> > On Oct 17, 2010, at 3:06 PM, Marcin Świderski wrote:
> >
> >> I'm currently working on modeling destructors of temporary objects in
> CFG. I've attached patch with prototype implementation and some tests.
> >>
> >
> > Hi Marcin,
> >
> > Aside from your other comment (which I will get to later), I had a
> question about the following:
> >
> > +CFGBlock
> *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E) {
> > +  if (E->isLogicalOp()) {
> > +    // Destructors for temporaries in LHS expression should be called
> after
> > +    // those for RHS expression. Even if this will unnecessarily create
> a block,
> > +    // this block will be used at least by the full expression.
> >
> > 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.
> >
> > Ted
>
> More specifically, my comment applies to the following code:
>
> +CFGBlock *CFGBuilder::VisitBinaryOperatorForTemporaryDtors(BinaryOperator
> *E) {
> +  if (E->isLogicalOp()) {
> +    // Destructors for temporaries in LHS expression should be called
> after
> +    // those for RHS expression. Even if this will unnecessarily create a
> block,
> +    // this block will be used at least by the full expression.
>
> 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?


I agree. So  the fix would be in CFGBlock *CFGBuilder::VisitChildren(Stmt*
Terminator), visit the children in reverse order.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20101022/4be9e438/attachment.html>