[PATCH] [ms-cxxabi] Destroy temporary record arguments in the callee

[John McCall]

On Jun 19, 2013, at 1:40 PM, Reid Kleckner <rnk at google.com> wrote:
> On Wed, Jun 19, 2013 at 3:01 PM, John McCall <rjmccall at apple.com> wrote:
> If the cleanup doesn't actually get popped off the cleanup stack, it will still be around for later code in the full-expression.
> 
> Here, I think this test case should work:
> 
>   struct A { A(); ~A(); };
>   int foo(A a, const A &a);
>   int bar();
>   ...
>   int baz(bool cond) {
>     return (cond ? foo(A(), A()) : bar());
>   }
> 
> Also, you should pop your cleanups in reverse order to make it more likely that they actually pop off.
> 
> Now that I deactivate in reverse order, the cleanups are getting popped, and everything is much cleaner.  I had to add more tests to cover the case where I actually deactivate a cleanup in a conditional.

Not just in a conditional, but specifically in a conditional when you can't pop the cleanups because another (unpoppable) cleanup has been added.  And if you want to be really good, make sure that a landing pad for that cleanup has been created by introducing a call after it's pushed but before the outer call tries to deactivate cleanups.

> I think the placeholder DominatingIP is OK as is because of this code in SetupCleanupBlockActivation:
> 
>     // If we're in a conditional block, ignore the dominating IP and
>     // use the outermost conditional branch.
>     if (CGF.isInConditionalBranch()) {
>       CGF.setBeforeOutermostConditional(value, var);
>     } else {
>       new llvm::StoreInst(value, var, dominatingIP);
>     }
> 
> Does that make sense?  Basically, the conditional case is already handled for me.  The cleanup is created in a deactivated state, activated when I push it, and then deactivated before the call.

Ah, yes, you're right.  So the placeholders are unnecessary in this case but do no harm.

> Anyway, this has tests now.  I wasn't sure how carefully I should match the labels from the landing pads in the lit tests, but they look correct from inspection.

Yeah, unfortunately matching control flow in LLVM IR with FileCheck is very challenging.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130619/2df21f48/attachment.html>