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

[Reid Kleckner]

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.

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.

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.

Contrariwise, MSVC presumably accepts this closely-related test case, which
> we would reject:
>   class A {
>     ~A();
>     friend void bar(A *);
>   };
>   void bar(A a) { }
>   void foo(A *a) { bar(*a); }
>
> This is not just an ABI difference;  it is a significant change to the
> language, and we should implement its rules correctly.
>

OK, sounds good.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130619/e1d75b26/attachment.html>