On Tue, Jun 24, 2014 at 6:21 PM, Manuel Klimek <[email protected]> wrote:
> On Tue, Jun 24, 2014 at 6:08 PM, Jordan Rose <[email protected]> > wrote: > >> >> On Jun 19, 2014, at 22:02 , Manuel Klimek <[email protected]> wrote: >> >> On Fri, Jun 20, 2014 at 5:28 AM, Jordan Rose <[email protected]> >> wrote: >> >>> >>> On Jun 19, 2014, at 14:19 , Manuel Klimek <[email protected]> wrote: >>> >>> On Thu, Jun 19, 2014 at 6:30 PM, Jordan Rose <[email protected]> >>> wrote: >>> >>>> I think the algorithm makes sense. I'm not sure it's different, though, >>>> than just passing up the first (or last) CXXBindTemporaryExpr visited for a >>>> given expression, which would look like this: >>>> >>>> // For a logical expression... >>>> VisitForTemporaryDtors(E->getLHS(), false, &LastBTE); >>>> const CXXBindTemporaryExpr *InnerBTE = nullptr; >>>> VisitForTemporaryDtors(E->getRHS(), false, &InnerBTE); >>>> InsertTempDtorDecisionBlock(InnerBTE); >>>> >>>> Are there any cases that wouldn't cover? >>>> >>> >>> Well, the problem is what to do if we don't have a BTE yet; if we only >>> wanted to pass one pointer, the semantics on function exit would need to be: >>> if (BTE) { we have already found a BTE, no need to insert a new block >>> when we encounter the next } >>> else { we have not yet found a BTE, so we need to insert a new block >>> when we find one } >>> >>> The "unconditional" branch would only fit with the first part, so we >>> would need to somehow conjure non-null BTE for all unconditional entries, >>> and then afterwards know that this is a special value, because since we >>> didn't add an extra block (as we were running an unconditional case), we >>> don't need a decision block. >>> I'd say that's a pretty strong argument that we at least need to pass >>> the CXXBindTemporaryExpr* and a bool IsConditional. >>> >>> Now it's right that we don't need to remember the Succ when we hit the >>> conditional, and instead we could just always store the Succ when we enter >>> a recursive visitation for a conditional branch (we need to store the Succ >>> because we can have nested conditionals), but that seems to me like it >>> would distribute the logic through even more places, and thus undesirable. >>> >>> >>> My observation is that only certain expressions cause conditional >>> branching, and that for these expressions you *always* need to >>> introduce a new block if you find any destructors, say, in the RHS of a >>> logical expression. So >>> >>> 1. if you're in a non-conditional sub-expression, it doesn't matter >>> whether there are temporaries or not; you don't need to insert a decision >>> branch, >>> 2. if you're in a conditional sub-expression and there are no >>> temporaries, you don't need to insert a decision branch, >>> 3. if you're in a conditional sub-expression and there are temporaries, >>> you can use any temporary from that subexpression as the guard. >>> >> >> That is exactly the algorithm. >> >> >>> So it looks to me like the only information you have to pass up from >>> traversing the sub-expressions is a BTE from that subexpression. Everything >>> else can be handled at the point where you start processing that >>> subexpression. >>> >> >> We have to pass the information down whether we are in a conditional >> part, so we know whether we have to start a new block when we hit the >> temporary. >> >> If you're asking why we cannot start the block at the conditional point, >> the reason is that we cannot add it before we do the subexpression >> traversal (because we don't know yet whether there will be temporaries, and >> we don't want to add a block if there are no temporaries), and if we want >> to do it after the subexpression traversal, we'd somehow need to split >> blocks (as there can be nested conditionals, and multiple temporaries). >> >> >> Hm. So in order to add the condition after the subexpression, we'd have >> to always start a new block *before* the subexpression. That actually >> feels a bit cleaner to me, though—start a new block, traverse the >> subexpression, and if the block is empty, throw it away and go back to the >> block we had before. Having less things in flight like that makes me a bit >> less nervous about maintaining this code in the future. If you disagree, >> though, then what you have looks about as clean as it can get. >> > > The problem is when we hit nested branches, I think: > b || (b || t()) > We hit the first ||, we add an empty block. We hit the second ||, we add > an empty block. We visit t() and add it to that empty block. Pop the stack, > see that we need a decision block - now we hook up the decision block to > the empty block. Pop the stack. Now we have to somehow wind the empty block > out of the generated structure, and hook it up correctly to the previous > block. I'd expect that to be less maintainable than the current solution. > > >> Uh, one other problem: even though we're now sharing branches for several >> temporaries, we still have to clean up state for every >> CXXBindTemporaryExpr. That should probably be moved out >> of processCleanupTemporaryBranch and into ProcessTemporaryDtor. >> > > I'll do that. > Done. I had to shuffle methods around a bit... > > Cheers, > /Manuel >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
