Hi Jason,
>>+ tree iarc_m = lookup_member (frame_type, coro_frame_i_a_r_c_id,
>>+ 1, 0, tf_warning_or_error);
>>+ tree iarc_x = build_class_member_access_expr (deref_fp, iarc_m,
>>NULL_TREE,
>>+ false, tf_warning_or_error);
>Do you need to call lookup_member directly? I'd think you could just pass the
>identifier to build_class_member_access_expr. And in several other places in
>coroutine.cc, as well.
AFAICT, I do... the doc for build_class_member_access_expr () says:
"MEMBER is a DECL or baselink.". We could extend the API to allow a member
identifier (which would be nice). Maybe I'm missing something else?
>> /* We always expect to delete the frame. */
>This seems to no longer be true after this patch?
Updated the comments.
>>+ fnf_if = do_poplevel (fnf_if_scope);
>>+ add_stmt (fnf_if);
>Why not finish_if_stmt instead of these 4 lines?
Actually, I have no recollection of why they are like that, some fallout of
the evolution of the code...
Amended to use finish_if_stmt() instead.
retested on x86_64-darwin, OK for trunk?
thanks
Iain
--- 8< ---
The responsibility for destroying part of the frame content (promise,
arg copies and the frame itself) transitions from the ramp to the
body of the coroutine once we reach the await_resume () for the
initial suspend.
We added the variable that flags the transition, but failed to act on
it. This corrects that so that the ramp only tries to run DTORs for
objects when an exception occurs before the initial suspend await
resume has started.
PR c++/113773
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::build_ramp_function): Only cleanup the
frame state on exceptions that occur before the initial await
resume has begun.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/torture/pr113773.C: New test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 39 +++++++----
.../g++.dg/coroutines/torture/pr113773.C | 66 +++++++++++++++++++
2 files changed, 92 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/pr113773.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 8cdc76a6054..2f128db42e1 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -5129,12 +5129,22 @@ cp_coroutine_transform::build_ramp_function ()
finish_if_stmt_cond (coro_gro_live, gro_d_if);
finish_expr_stmt (gro_ret_dtor);
finish_then_clause (gro_d_if);
- tree gro_d_if_scope = IF_SCOPE (gro_d_if);
- IF_SCOPE (gro_d_if) = NULL;
- gro_d_if = do_poplevel (gro_d_if_scope);
- add_stmt (gro_d_if);
+ finish_if_stmt (gro_d_if);
}
+ /* Before initial resume is called, the responsibility for cleanup on
+ exception falls to the ramp. After that, the coroutine body code
+ should do the cleanup. */
+ tree iarc_m = lookup_member (frame_type, coro_frame_i_a_r_c_id,
+ 1, 0, tf_warning_or_error);
+ tree iarc_x
+ = build_class_member_access_expr (deref_fp, iarc_m, NULL_TREE,
+ /*preserve_reference*/false,
+ tf_warning_or_error);
+ tree not_iarc
+ = build1_loc (loc, TRUTH_NOT_EXPR, boolean_type_node, iarc_x);
+ tree cleanup_if = begin_if_stmt ();
+ finish_if_stmt_cond (not_iarc, cleanup_if);
/* If the promise is live, then run its dtor if that's available. */
if (promise_dtor && promise_dtor != error_mark_node)
{
@@ -5142,10 +5152,7 @@ cp_coroutine_transform::build_ramp_function ()
finish_if_stmt_cond (coro_promise_live, promise_d_if);
finish_expr_stmt (promise_dtor);
finish_then_clause (promise_d_if);
- tree promise_d_if_scope = IF_SCOPE (promise_d_if);
- IF_SCOPE (promise_d_if) = NULL;
- promise_d_if = do_poplevel (promise_d_if_scope);
- add_stmt (promise_d_if);
+ finish_if_stmt (promise_d_if);
}
/* Clean up any frame copies of parms with non-trivial dtors.
@@ -5169,15 +5176,21 @@ cp_coroutine_transform::build_ramp_function ()
finish_if_stmt_cond (parm_i->guard_var, dtor_if);
finish_expr_stmt (parm_i->fr_copy_dtor);
finish_then_clause (dtor_if);
- tree parm_d_if_scope = IF_SCOPE (dtor_if);
- IF_SCOPE (dtor_if) = NULL;
- dtor_if = do_poplevel (parm_d_if_scope);
- add_stmt (dtor_if);
+ finish_if_stmt (dtor_if);
}
}
- /* We always expect to delete the frame. */
+ /* No delete the frame if required. */
+ tree fnf_if = begin_if_stmt ();
+ finish_if_stmt_cond (fnf_x, fnf_if);
finish_expr_stmt (delete_frame_call);
+ finish_then_clause (fnf_if);
+ finish_if_stmt (fnf_if);
+
+ /* Finished cleanups conditional on "initial resume is not called". */
+ finish_then_clause (cleanup_if);
+ finish_if_stmt (cleanup_if);
+
tree rethrow = build_throw (loc, NULL_TREE, tf_warning_or_error);
suppress_warning (rethrow);
finish_expr_stmt (rethrow);
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr113773.C
b/gcc/testsuite/g++.dg/coroutines/torture/pr113773.C
new file mode 100644
index 00000000000..b048b0d63f4
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr113773.C
@@ -0,0 +1,66 @@
+// { dg-do run }
+#include <coroutine>
+#ifdef OUTPUT
+#include <iostream>
+#endif
+
+struct result {
+ operator int() {
+ throw 42;
+ }
+};
+
+static int p_dtor_count = 0;
+class promise {
+public:
+ result get_return_object() { return {}; }
+ std::suspend_never initial_suspend() {
+#ifdef OUTPUT
+ std::cout << "initial suspend" << std::endl;
+#endif
+ return {};
+ }
+ void unhandled_exception() {
+#ifdef OUTPUT
+ std::cout << "unhandled exception" << std::endl;
+#endif
+ }
+ std::suspend_never final_suspend() noexcept {
+#ifdef OUTPUT
+ std::cout << "final suspend" << std::endl;
+#endif
+ return {};
+ }
+ void return_void() {}
+ ~promise() {
+ p_dtor_count++;
+#ifdef OUTPUT
+ std::cout << "~promise()" << std::endl;
+#endif
+ }
+};
+
+template <class... Args>
+struct std::coroutine_traits<int, Args...> {
+ using promise_type = promise;
+};
+
+int f() {
+ co_return;
+}
+
+int main() {
+ try {
+ f();
+ }
+ catch (int i) {
+ if (i != 42)
+ __builtin_abort ();
+#ifdef OUTPUT
+ std::cout << "caught 42" << std::endl;
+#endif
+ }
+ if (p_dtor_count != 1)
+ __builtin_abort ();
+ return 0;
+}
--
2.39.2 (Apple Git-143)
