[PATCH] Proposal on how to fix temporary dtors.

[Manuel Klimek]

On Wed, May 28, 2014 at 5:23 PM, Jordan Rose <jordan_rose at apple.com> wrote:

> We should treat blocks the same as lambdas. How about GNU StmtExprs?
>
>   if (check(OtherDtor()) &&
>       ({ int x = 1; NoReturnDtor(); x + 1})) { ... }
>
> To be fair, I'm not even sure how those are //supposed// to behave.
>

I'll investigate more.

I'm not sure how your last-bind trick really works. In this code:


>   First() && (Second() || Second()) && Third()
>
> none of the temporaries can be aggregated, but I'm not sure how
> LastBoundTemporary accomplishes that, even given your explanation.
>

/me headdesks. Yes, it's wrong. I'll come up with a (hopefully) correct
solution (see below for why).


>
> Can we start with the inefficient version that tests every temporary
> individually, and then consolidate afterwards?
>

Unfortunately that breaks invariants in the simple path-based reachability
analysis, which then gives pretty bad false positives:
int f() {
  check(NoReturnDtor()) || somethingElse();
}
will trigger the "return from non-void function" warning, as with the
simple version we now have a path running to the end of the function.

It also led to performance regressions in the analyzer with many temp dtors
happening...

Thanks for your patience in reviewing my attempts!
Cheers,
/Manuel


>
> ================
> Comment at: test/Analysis/temporaries.cpp:336-338
> @@ +335,5 @@
> +      ++y;
> +      // Test that the CFG gets hooked up correctly when temporary
> destructors
> +      // are handled after a statically known branch condition.
> +      true ? (void)0 : (void)check(NoReturnDtor());
> +    }
> ----------------
> If this is the right test, then the comment was all I really wanted. There
> is of course a difference between this and the if-statement — the
> destructor here is in the same full-expression as the condition.
>
> http://reviews.llvm.org/D3627
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140528/0fd077d9/attachment.html>