On 9/17/26 6:58 PM, Joshua Berne wrote:
On Thu, Sep 17, 2026 at 6:36 PM Jason Merrill <[email protected]
<mailto:[email protected]>> wrote:
On 9/15/26 10:42 PM, Joshua Berne wrote:
> Attached is a patch for PR127281. I added testing to an existing
test
> in order to explore more adjacent corners and capture a runtime
failure
> that was hiding behind the ICE. When there were two effective
blocks in
> a function the compiler ICEd because of two conflicting
declarations for
> the return value cleanup sentinel. With no intervening declaration,
> there were two destructors calls if an exception escaped the
destruction
> of temporaries.
>
> There's another followup bug (127414) that requires additional
cleanup
> steps to be added to handle proper exceptions escaping from
> postcondition evaluations. Right now we leak or double destroy,
this
> fix makes it all just leak, and the fix to 127414 looks like it will
> depend on this fix as well.
>
> I ran the full dg test suite (check=g++ and check-gcc) before and
> aftetr, no new tests failed and 3 new tests passed (the runtime
testing
> that i added for the existing test).
Hmm, this seems like a bandaid until the future 127414 patch actually
fixes the interaction between contracts and the return cleanup. I'm
open to applying it anyway, but both the commit message and the code
comment should make that clear.
I think it's more of a subset of that fix (that is separable) than a
bandaid. The 127414 fix seems like it will require adding in the
cleanup guards in the right places around the created set of contract
evaluation statements, (in 2 places) and that still requires supressing
the extra adding of those guards by do_poplevel (which will just
inevitably ice or double-destroy if left there).
I imagined it moving the contract evaluation inside the existing guards
rather than duplicating them, but in any case it's true that you need to
avoid duplicate guards somehow, and this is a straightforward way to do
that.
This fix also impacts anyone using contracts (due to the ICE), while
127414 will only impact those using a throwing violation handler.
Either way, i'm happy to improve the commit message and comment, should
i reference 127414 in both?
Well, it's already in the commit message, so just need to add it to the
comment.
Jason
