On 5/13/25 10:30 AM, Iain Sandoe wrote:
The revised wording for coroutines, uses decltype(auto) for the
type of the get return object, which preserves references. The
test is expected to fail, since it attempts to initialize the
return object from an object that has already been destroyed.
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::build_ramp_function): Use
decltype(auto) to determine the type of the temporary
get_return_object.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr115908.C: Count promise construction
and destruction.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 22 ++++---
gcc/testsuite/g++.dg/coroutines/pr115908.C | 69 +++++++++++++++-------
2 files changed, 60 insertions(+), 31 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 42f6e32e89c..ce3e022a516 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -5120,8 +5120,11 @@ cp_coroutine_transform::build_ramp_function ()
/* Check for a bad get return object type.
[dcl.fct.def.coroutine] / 7 requires:
The expression promise.get_return_object() is used to initialize the
- returned reference or prvalue result object ... */
- tree gro_type = TREE_TYPE (get_ro);
+ returned reference or prvalue result object ...
+ When we use a local to hold this, it is decltype(auto). */
+ tree gro_type
+ = finish_decltype_type (get_ro, /*id_expression_or_member_access_p*/true,
This should be false, not true; a call is not an id-expr or member access.
+ tf_warning_or_error); // TREE_TYPE (get_ro);
if (VOID_TYPE_P (gro_type) && !void_ramp_p)
{
error_at (fn_start, "no viable conversion from %<void%> provided by"
@@ -5129,11 +5132,6 @@ cp_coroutine_transform::build_ramp_function ()
return false;
}
- /* 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);
Moving this initialization doesn't seem connected to the type of gro, or
mentioned above?
/* [dcl.fct.def.coroutine] / 7
The expression promise.get_return_object() is used to initialize the
glvalue result or prvalue result object of a call to a coroutine. */
@@ -5153,7 +5151,7 @@ cp_coroutine_transform::build_ramp_function ()
= coro_build_and_push_artificial_var (loc, "_Coro_gro", gro_type,
orig_fn_decl, NULL_TREE);
- r = cp_build_init_expr (coro_gro, get_ro);
+ r = cp_build_init_expr (coro_gro, STRIP_REFERENCE_REF (get_ro));
finish_expr_stmt (r);
tree coro_gro_cleanup
= cxx_maybe_build_cleanup (coro_gro, tf_warning_or_error);
@@ -5161,6 +5159,11 @@ cp_coroutine_transform::build_ramp_function ()
push_cleanup (coro_gro, coro_gro_cleanup, /*eh_only*/false);
}
+ /* 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);
+
/* Start the coroutine body. */
r = build_call_expr_loc (fn_start, resumer, 1, coro_fp);
finish_expr_stmt (r);
@@ -5179,7 +5182,8 @@ 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 function body. */
- finish_return_stmt (void_ramp_p ? NULL_TREE : coro_gro);
+ r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
+ finish_return_stmt (r);
if (flag_exceptions)
{
diff --git a/gcc/testsuite/g++.dg/coroutines/pr115908.C
b/gcc/testsuite/g++.dg/coroutines/pr115908.C
index ac27d916de2..6956c83a8df 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr115908.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr115908.C
@@ -6,23 +6,28 @@
struct Promise;
-bool promise_live = false;
+int promise_life = 0;
struct Handle : std::coroutine_handle<Promise> {
+
+#if 1
+ /* We now expect the handle to be created after the promise is destroyed.
*/
Yes, though this is terrible, as noted in my email to core today. Why
doesn't this also break folly::Optional/Expected?
This shouldn't block this patch series, but ISTM we'll want something
further to address this issue.
Handle(Promise &p) :
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
- if (!promise_live)
- __builtin_abort ();
#ifdef OUTPUT
- std::cout << "Handle(Promise &)\n";
+ std::cout << "Handle(Promise &) " << promise_life << std::endl;
#endif
- }
- Handle(Promise &&p) :
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
- if (!promise_live)
+ if (promise_life <= 0)
__builtin_abort ();
+ }
+#endif
+
+ Handle(Promise &&p) :
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
#ifdef OUTPUT
- std::cout << "Handle(Promise &&)\n";
+ std::cout << "Handle(Promise &&) " << promise_life << std::endl;
#endif
- }
+ if (promise_life <= 0)
+ __builtin_abort ();
+ }
using promise_type = Promise;
};
@@ -30,46 +35,66 @@ struct Handle : std::coroutine_handle<Promise> {
struct Promise {
Promise() {
#ifdef OUTPUT
- std::cout << "Promise()\n";
+ std::cout << "Promise()" << std::endl;
#endif
- promise_live = true;
+ promise_life++;
}
Please add a Promise copy constructor, whether deleted or defined so
that promise_life is accurate if it's called.
~Promise() {
#ifdef OUTPUT
- std::cout << "~Promise()\n";
+ std::cout << "~Promise()" << std::endl;
#endif
- if (!promise_live)
+ if (promise_life <= 0)
__builtin_abort ();
- promise_live = false;
+ promise_life--;
}
+
Promise& get_return_object() noexcept {
#ifdef OUTPUT
- std::cout << "get_return_object()\n";
+ std::cout << "get_return_object() " << promise_life << std::endl;
#endif
- if (!promise_live)
+ if (promise_life <= 0)
__builtin_abort ();
return *this;
}
- std::suspend_never initial_suspend() const noexcept { return {}; }
- std::suspend_never final_suspend() const noexcept { return {}; }
+
+ std::suspend_never initial_suspend() const noexcept {
+#ifdef OUTPUT
+ std::cout << "initial_suspend()" << std::endl;
+#endif
+ return {};
+ }
+ std::suspend_never final_suspend() const noexcept {
+#ifdef OUTPUT
+ std::cout << "final_suspend()" << std::endl;
+#endif
+ return {};
+ }
void return_void() const noexcept {
- if (!promise_live)
+ if (!promise_life)
__builtin_abort ();
#ifdef OUTPUT
- std::cout << "return_void()\n";
+ std::cout << "return_void()" << std::endl;
#endif
}
void unhandled_exception() const noexcept {}
};
Handle Coro() {
+
+#ifdef OUTPUT
+ std::cout << "Coro()" << std::endl;
+#endif
co_return;
}
int main() {
- Coro();
- if (promise_live)
+ Coro();
+#ifdef OUTPUT
+ std::cout << "done Coro()" << std::endl;
+#endif
+ if (promise_life)
__builtin_abort ();
return 0;
}
