This is needed to be able to back-port the rest of the CWG2563 work to
GCC-15.2, so I've prioritised it. So far, tested on x86_64 darwin, OK
for trunk (and backport) if it passes wider testing?
thanks
Iain
--- 8< ---
The current implementation was returning the result of the g_r_o_o_a_f
call independently of the return expressions for 'normal' cases.
This prevents the NVRO that we need to guarantee copy elision for the
ramp return values - when these are initialised from a temporary of the
same type.
The solution here:
- pushes the g_r_o to the outermost scope of the ramp and makes it auto
- updates the type when we know it (after initialising the promise etc)
- in the case that the allocation fails, and we have a g_r_o_o_a_f, we
then determine if the type of the g_r_o is the same as the type of the
ramp return. If this is the case, then we must initialize the g_r_o
from the g_r_o_o_a_f (and the process will fail if this is not well
formed). In the case that the types of the ramp return and the g_r_o
differ then there is no copy elision and we can just use the normal
return machinery to return the g_r_o_o_a_f.
PR c++/121219
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::build_ramp_function): Ensure tha we
initialise and return g_r_o from any provided g_r_o_o_a_f if
the ramp return type is the same as the g_r_o type.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/torture/pr121219.C: New test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 77 ++++++---
.../g++.dg/coroutines/torture/pr121219.C | 147 ++++++++++++++++++
2 files changed, 201 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index e9068c11063..d2716bd1397 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -5141,10 +5141,19 @@ cp_coroutine_transform::build_ramp_function ()
tree r = cp_build_init_expr (coro_fp, allocated);
finish_expr_stmt (r);
+ /* Temporary var to hold the g_r_o across the function body. */
+ tree coro_gro = NULL_TREE;
+ tree gro_type = NULL_TREE;
+ if (!void_ramp_p)
+ coro_gro
+ = coro_build_and_push_artificial_var (loc, "_Coro_gro", make_auto (),
+ orig_fn_decl, NULL_TREE);
+
/* If the user provided a method to return an object on alloc fail, then
check the returned pointer and call the func if it's null.
Otherwise, no check, and we fail for noexcept/fno-exceptions cases. */
+ tree grooaf_if_stmt = NULL_TREE;
if (grooaf)
{
/* [dcl.fct.def.coroutine] / 10 (part 3)
@@ -5152,20 +5161,10 @@ cp_coroutine_transform::build_ramp_function ()
control to the caller of the coroutine and the return value is
obtained by a call to T::get_return_object_on_allocation_failure(),
where T is the promise type. */
- tree if_stmt = begin_if_stmt ();
tree cond = build1 (CONVERT_EXPR, frame_ptr_type, nullptr_node);
- cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond);
- finish_if_stmt_cond (cond, if_stmt);
- r = NULL_TREE;
- if (void_ramp_p)
- /* Execute the get-return-object-on-alloc-fail call... */
- finish_expr_stmt (grooaf);
- else
- /* Get the fallback return object. */
- r = grooaf;
- finish_return_stmt (r);
- finish_then_clause (if_stmt);
- finish_if_stmt (if_stmt);
+ cond = build2 (NE_EXPR, boolean_type_node, coro_fp, cond);
+ grooaf_if_stmt = begin_if_stmt ();
+ finish_if_stmt_cond (cond, grooaf_if_stmt);
}
/* Dereference the frame pointer, to use in member access code. */
@@ -5355,7 +5354,7 @@ cp_coroutine_transform::build_ramp_function ()
The expression promise.get_return_object() is used to initialize the
returned reference or prvalue result object ...
When we use a local to hold this, it is decltype(auto). */
- tree gro_type
+ gro_type
= finish_decltype_type (get_ro, /*id_expression_or_member_access_p*/false,
tf_warning_or_error);
if (VOID_TYPE_P (gro_type) && !void_ramp_p)
@@ -5364,8 +5363,13 @@ cp_coroutine_transform::build_ramp_function ()
" %<get_return_object%> to return type %qT", fn_return_type);
return false;
}
+ else if (!void_ramp_p)
+ {
+ TREE_TYPE (coro_gro) = gro_type;
+ relayout_decl (coro_gro);
+ }
- /* Initialize the resume_idx_var to 0, meaning "not started". */
+/* Initialize the resume_idx_var to 0, meaning "not started". */
coro_build_and_push_artificial_var_with_dve
(loc, coro_resume_index_id, short_unsigned_type_node, orig_fn_decl,
build_zero_cst (short_unsigned_type_node), deref_fp);
@@ -5380,7 +5384,6 @@ cp_coroutine_transform::build_ramp_function ()
The expression promise.get_return_object() is used to initialize the
glvalue result or prvalue result object of a call to a coroutine. */
- tree coro_gro = NULL_TREE;
if (void_ramp_p)
/* We still want to call the method, even if the result is unused. */
finish_expr_stmt (get_ro);
@@ -5390,11 +5393,6 @@ cp_coroutine_transform::build_ramp_function ()
a temp which is then used to intialize the return object, including
NVRO. */
- /* Temporary var to hold the g_r_o across the function body. */
- coro_gro
- = coro_build_and_push_artificial_var (loc, "_Coro_gro", gro_type,
- orig_fn_decl, NULL_TREE);
-
r = cp_build_init_expr (coro_gro, STRIP_REFERENCE_REF (get_ro));
finish_expr_stmt (r);
tree coro_gro_cleanup
@@ -5423,8 +5421,41 @@ cp_coroutine_transform::build_ramp_function ()
/* The ramp is done, we just need the return statement, which we build from
the return object we constructed before we called the actor. */
- r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
- finish_return_stmt (r);
+ if (grooaf)
+ {
+ /* This is our 'normal' exit. */
+ r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
+ finish_return_stmt (r);
+ finish_then_clause (grooaf_if_stmt);
+
+ begin_else_clause (grooaf_if_stmt);
+ /* We come here if the frame allocation failed. */
+ if (void_ramp_p)
+ {
+ /* Execute the get-return-object-on-alloc-fail call... */
+ finish_expr_stmt (grooaf);
+ finish_return_stmt (NULL_TREE);
+ }
+ else
+ {
+ /* Get the fallback return object. */
+ if (same_type_p (gro_type, fn_return_type))
+ {
+ /* Copy elision via NVRO should be in force. */
+ r = cp_build_init_expr (coro_gro, grooaf);
+ finish_expr_stmt (r);
+ finish_return_stmt (convert_from_reference (coro_gro));
+ }
+ else
+ finish_return_stmt (grooaf);
+ }
+ finish_if_stmt (grooaf_if_stmt);
+ }
+ else
+ {
+ r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
+ finish_return_stmt (r);
+ }
finish_compound_stmt (ramp_fnbody);
return true;
diff --git a/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
b/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
new file mode 100644
index 00000000000..4cb8c7ae7ea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/torture/pr121219.C
@@ -0,0 +1,147 @@
+// { dg-do run }
+
+#include <coroutine>
+#ifdef OUTPUT
+#include <iostream>
+#endif
+#include <stdexcept>
+
+struct Task {
+ struct promise_type;
+ using handle_type = std::coroutine_handle<promise_type>;
+
+ struct promise_type {
+ Task* task_;
+ int result_;
+
+ static void* operator new(std::size_t size) noexcept {
+ void* p = ::operator new(size, std::nothrow);
+#ifdef OUTPUT
+ std::cerr << "operator new (no arg) " << size << " -> " << p <<
std::endl;
+#endif
+ return p;
+ }
+ static void operator delete(void* ptr) noexcept {
+ return ::operator delete(ptr, std::nothrow);
+ }
+#if 1 // change to 0 to fix crash
+ static Task get_return_object_on_allocation_failure() noexcept {
+#ifdef OUTPUT
+ std::cerr << "get_return_object_on_allocation_failure" <<
std::endl;
+#endif
+ return Task(nullptr);
+ }
+#endif
+
+ auto get_return_object() {
+#ifdef OUTPUT
+ std::cerr << "get_return_object" << std::endl;
+#endif
+ return Task{handle_type::from_promise(*this)};
+ }
+
+ auto initial_suspend() {
+#ifdef OUTPUT
+ std::cerr << "initial_suspend" << std::endl;
+#endif
+ return std::suspend_always{};
+ }
+
+ auto final_suspend() noexcept {
+#ifdef OUTPUT
+ std::cerr << "final_suspend" << std::endl;
+#endif
+ return std::suspend_never{}; // Coroutine auto-destructs
+ }
+
+ ~promise_type() {
+ if (task_) {
+#ifdef OUTPUT
+ std::cerr << "promise_type destructor: Clearing Task handle"
<< std::endl;
+#endif
+ task_->h_ = nullptr;
+ }
+ }
+
+ void unhandled_exception() {
+#ifdef OUTPUT
+ std::cerr << "unhandled_exception" << std::endl;
+#endif
+ std::terminate();
+ }
+
+ void return_value(int value) {
+#ifdef OUTPUT
+ std::cerr << "return_value: " << value << std::endl;
+#endif
+ result_ = value;
+ if (task_) {
+ task_->result_ = value;
+ task_->completed_ = true;
+ }
+ }
+ };
+
+ handle_type h_;
+ int result_;
+ bool completed_ = false;
+
+ Task(handle_type h) : h_(h) {
+#ifdef OUTPUT
+ std::cerr << "Task constructor" << std::endl;
+#endif
+ if (h_) {
+ h_.promise().task_ = this; // Link promise to Task
+ }
+ }
+
+ ~Task() {
+#ifdef OUTPUT
+ std::cerr << "~Task destructor" << std::endl;
+#endif
+ // Only destroy handle if still valid (coroutine not completed)
+ if (h_) {
+#ifdef OUTPUT
+ std::cerr << "Destroying coroutine handle" << std::endl;
+#endif
+ h_.destroy();
+ }
+ }
+
+ bool done() const {
+ return completed_ || !h_ || h_.done();
+ }
+
+ void resume() {
+#ifdef OUTPUT
+ std::cerr << "Resuming task" << std::endl;
+#endif
+ if (h_) h_.resume();
+ }
+
+ int result() const {
+ if (!done()) throw std::runtime_error("Result not available");
+ return result_;
+ }
+};
+
+Task my_coroutine() {
+#ifdef OUTPUT
+ std::cerr << "Inside my_coroutine" << std::endl;
+#endif
+ co_return 42;
+}
+
+int main() {
+ auto task = my_coroutine();
+ while (!task.done()) {
+#ifdef OUTPUT
+ std::cerr << "Resuming task in main" << std::endl;
+#endif
+ task.resume();
+ }
+#ifdef OUTPUT
+ std::cerr << "Task completed in main, printing result" << std::endl;
+#endif
+ return task.result();
+}
--
2.39.2 (Apple Git-143)
