On Jun 24, 2014, at 9:21 , 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.
Okay. I feel like the CFG builder already has to do things like this, but maybe not in this particular direction. Let's stick with what you have. Jordan
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
