On 8/22/24 11:42 AM, Jason Merrill wrote:
On 8/21/24 3:10 PM, Iain Sandoe wrote:
Require that the value returned by get_return_object is convertible to
the ramp return. This means that the only time we allow a void
get_return_object, is when the ramp is also a void function.
We diagnose this early to allow us to exit the ramp build if the return
values are incompatible.
PR c++/100476
gcc/cp/ChangeLog:
* coroutines.cc
(cp_coroutine_transform::build_ramp_function): Remove special
handling of void get_return_object expressions.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/coro-bad-gro-01-void-gro-non-class-coro.C:
Adjust expected diagnostic.
* g++.dg/coroutines/pr102489.C: Avoid void get_return_object.
* g++.dg/coroutines/pr103868.C: Likewise.
* g++.dg/coroutines/pr94879-folly-1.C: Likewise.
* g++.dg/coroutines/pr94883-folly-2.C: Likewise.
* g++.dg/coroutines/pr96749-2.C: Likewise.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
gcc/cp/coroutines.cc | 48 +++++++++----------
.../coro-bad-gro-01-void-gro-non-class-coro.C | 2 +-
gcc/testsuite/g++.dg/coroutines/pr102489.C | 2 +-
gcc/testsuite/g++.dg/coroutines/pr103868.C | 2 +-
.../g++.dg/coroutines/pr94879-folly-1.C | 3 +-
.../g++.dg/coroutines/pr94883-folly-2.C | 39 +++++++--------
gcc/testsuite/g++.dg/coroutines/pr96749-2.C | 2 +-
7 files changed, 48 insertions(+), 50 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 2faf198c206..d152ad20dca 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -4640,6 +4640,7 @@ cp_coroutine_transform::build_ramp_function ()
tree promise_type = get_coroutine_promise_type (orig_fn_decl);
tree fn_return_type = TREE_TYPE (TREE_TYPE (orig_fn_decl));
+ bool void_ramp_p = VOID_TYPE_P (fn_return_type);
/* [dcl.fct.def.coroutine] / 10 (part1)
The unqualified-id get_return_object_on_allocation_failure is
looked up
@@ -4720,6 +4721,19 @@ cp_coroutine_transform::build_ramp_function ()
return;
}
+ /* Check for a bad get return object type. */
+ tree gro_return_type = FUNC_OR_METHOD_TYPE_P (TREE_TYPE (get_ro_meth))
+ ? TREE_TYPE (TREE_TYPE (get_ro_meth))
+ : TREE_TYPE (get_ro_meth);
Is this to allow get_return_type to be a function-like-object? If so,
checking its TREE_TYPE won't give the return type of its op(). Shouldn't
we just use TREE_TYPE (get_ro) once we have it?
+ if (VOID_TYPE_P (gro_return_type) && !void_ramp_p)
+ {
+ error_at (fn_start, "no viable conversion from %<void%>
provided by"
+ " %<get_return_object%> to return type %qT", fn_return_type);
+ valid_coroutine = false;
+ input_location = save_input_loc;
+ return;
+ }
+
/* So now construct the Ramp: */
tree stmt = begin_function_body ();
/* Now build the ramp function pieces. */
@@ -4816,7 +4830,7 @@ cp_coroutine_transform::build_ramp_function ()
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);
- if (VOID_TYPE_P (fn_return_type))
+ if (void_ramp_p)
{
/* Execute the get-return-object-on-alloc-fail call... */
finish_expr_stmt (grooaf);
@@ -5028,7 +5042,6 @@ cp_coroutine_transform::build_ramp_function ()
tree gro_context_body = push_stmt_list ();
tree gro_type = TREE_TYPE (get_ro);
- bool gro_is_void_p = VOID_TYPE_P (gro_type);
tree gro = NULL_TREE;
tree gro_bind_vars = NULL_TREE;
@@ -5037,8 +5050,11 @@ cp_coroutine_transform::build_ramp_function ()
tree gro_cleanup_stmt = NULL_TREE;
/* We have to sequence the call to get_return_object before initial
suspend. */
- if (gro_is_void_p)
- r = get_ro;
+ if (void_ramp_p)
+ {
+ gcc_checking_assert (VOID_TYPE_P (gro_type));
+ r = get_ro;
+ }
else if (same_type_p (gro_type, fn_return_type))
{
/* [dcl.fct.def.coroutine] / 7
...
/* ... or ... Construct an object that will be used as the
single param to the CTOR for the return object. */
You aren't touching this code in this patch, nor does this suggestion
need to be addressed in this patch, but is there a reason not to
construct the return value directly in this case as well? p7 only says
that the call to get_return_object is sequenced before initial_suspend,
but it doesn't say to delay the initialization of the return value, and
it does say that the call expression is used to initialize the return
value, which might have different semantics from the current
implementation with an intermediate variable.
...and now I see this is patch 7, I guess I really should read the whole
patch series before sending comments.
My earlier comment about gro_return_type still applies, however.
Jason
