On 6/11/25 1:12 PM, Iain Sandoe wrote:
On 11 Jun 2025, at 17:53, Jason Merrill <ja...@redhat.com> wrote:
On 6/9/25 3:54 PM, Iain Sandoe wrote:
I was planning to apply this as obvious - but it is needed for the
next patch to be posted - so noting here now. I discussed with one
of the original coroutines paper authors the idea that, if the original
function was marked noexcept, then we should carry that onto the
outlined body. This was not the design intent, so it is quite expected
that the ramp might be noexcept - but that a resumer via a handle could
still receive an exception. Given this and the fact we unconditionally
add exceptions code to the body, I consider this patch to be obvious and
the EH code certainly gets very confused without it (fun to debug).
Any comments?
thanks
Iain
--- 8<---
We must flag that the resumer might throw (since the wrapping of the
original function body unconditionally adds a try-catch/rethrow). We
also add code that might throw - even when the original function body
would not.
OK, but maybe we shouldn't add the try/catch if the original function can't
throw?
Perhaps we could under the ’as-if’ rule… but
1. the standard does not say this..
1a when I discussed with one of the authors, the response was that this was not
an omission (because of other added stuff that might throw).
Of course we should also check the added stuff, as you suggest below.
2. we can check that the initial awaiter / initial suspend do not throw, but we
don’t
know about any other coroutine that is potentially continued from
await_suspend.
Surely if a continuation could throw, then await_suspend can throw?
Perhaps if we add the initial awaiter sequence and then test to see if the
resulting
body cannot throw … that would seem to be a valid ‘as if’.
I view this as a pretty useful improvement we could make - especially in
reduction
of codegen, but the trivial patch here is just to cater for the status quo,
fixing a
correcness issue there.
does the improvement seem useful to pursue?
Yes.
… enough to block this patch?
No, I meant my "OK" above as approving this patch. But please add a
FIXME or ??? about the improvement. OK with that adjustment.
thanks
Iain
gcc/cp/ChangeLog:
* coroutines.cc (build_actor_fn): Set can_throw.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 18c0a4812c4..d82fa46f208 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -2412,6 +2412,9 @@ build_actor_fn (location_t loc, tree coro_frame_type,
tree actor, tree fnbody,
bool spf = start_preparsed_function (actor, NULL_TREE, SF_PRE_PARSED);
gcc_checking_assert (spf);
gcc_checking_assert (cfun && current_function_decl && TREE_STATIC (actor));
+ if (flag_exceptions)
+ /* We, unconditionally, add a try/catch and rethrow. */
+ cp_function_chain->can_throw = true;
tree stmt = begin_function_body ();
tree actor_bind = build3 (BIND_EXPR, void_type_node, NULL, NULL, NULL);
