Ping
On Wed, Jun 25, 2014 at 2:25 PM, Manuel Klimek <[email protected]> wrote: > 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
