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.
@@ -5122,31 +5138,11 @@ cp_coroutine_transform::build_ramp_function ()
for an object of the return type. */
if (same_type_p (gro_type, fn_return_type))
- r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig_fn_decl);
- else if (!gro_is_void_p)
+ r = void_ramp_p ? NULL_TREE : DECL_RESULT (orig_fn_decl);
+ else
/* check_return_expr will automatically return gro as an rvalue via
treat_lvalue_as_rvalue_p. */
r = gro;
- else if (CLASS_TYPE_P (fn_return_type))
- {
- /* For class type return objects, we can attempt to construct,
- even if the gro is void. ??? Citation ??? c++/100476 */
- r = build_special_member_call (NULL_TREE,
- complete_ctor_identifier, NULL,
- fn_return_type, LOOKUP_NORMAL,
- tf_warning_or_error);
- r = build_cplus_new (fn_return_type, r, tf_warning_or_error);
- }
- else
- {
- /* We can't initialize a non-class return value from void. */
- error_at (fn_start, "cannot initialize a return object of type"
- " %qT with an rvalue of type %<void%>", fn_return_type);
- r = error_mark_node;
- valid_coroutine = false;
- input_location = save_input_loc;
- return;
- }
finish_return_stmt (r);
diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-01-void-gro-non-class-coro.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-01-void-gro-non-class-coro.C
index 85aadad93b2..a7e3f3d1ac7 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-01-void-gro-non-class-coro.C
+++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-01-void-gro-non-class-coro.C
@@ -27,7 +27,7 @@ struct std::coroutine_traits<R, HandleRef, T...> {
};
int
-my_coro (std::coroutine_handle<>& h) // { dg-error {cannot initialize a return
object of type 'int' with an rvalue of type 'void'} }
+my_coro (std::coroutine_handle<>& h) // { dg-error {no viable conversion from
'void' provided by 'get_return_object' to return type 'int'} }
{
PRINT ("coro1: about to return");
co_return;
diff --git a/gcc/testsuite/g++.dg/coroutines/pr102489.C
b/gcc/testsuite/g++.dg/coroutines/pr102489.C
index 0ef06daa211..15b85f4375b 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr102489.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr102489.C
@@ -9,7 +9,7 @@ struct footask {
std::suspend_never initial_suspend();
std::suspend_never final_suspend() noexcept;
void unhandled_exception();
- void get_return_object();
+ footask get_return_object();
};
std::suspend_always foo;
footask taskfun() { co_await foo; }
diff --git a/gcc/testsuite/g++.dg/coroutines/pr103868.C
b/gcc/testsuite/g++.dg/coroutines/pr103868.C
index fd05769db3d..0ce40b699ce 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr103868.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr103868.C
@@ -116,7 +116,7 @@ struct awaitable_frame_base {
template <typename T> auto await_transform(T a) { return a; }
};
template <> struct awaitable_frame<void> : awaitable_frame_base {
- void get_return_object();
+ awaitable<void> get_return_object();
};
} // namespace detail
} // namespace asio
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
index 6e091526fe7..2d886fd387d 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr94879-folly-1.C
@@ -39,9 +39,10 @@ public:
std::g initial_suspend();
l final_suspend() noexcept;
};
+class n;
class m : public j {
public:
- void get_return_object();
+ n get_return_object();
void unhandled_exception();
};
class n {
diff --git a/gcc/testsuite/g++.dg/coroutines/pr94883-folly-2.C
b/gcc/testsuite/g++.dg/coroutines/pr94883-folly-2.C
index 98c5a7e3eee..f12897e8690 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr94883-folly-2.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr94883-folly-2.C
@@ -16,25 +16,7 @@ struct b {
void await_resume();
};
} // namespace std
-
-template <typename d> auto ab(int ac, d ad) -> decltype(ad.e(ac));
-int f;
-class h {
- class j {
- public:
- bool await_ready() noexcept;
- void await_suspend(std::coroutine_handle<>) noexcept;
- void await_resume() noexcept;
- };
-
-public:
- void get_return_object();
- std::b initial_suspend();
- j final_suspend() noexcept;
- void unhandled_exception();
- template <typename g>
- auto await_transform (g c) { return ab(f, c); }
-};
+class h;
template <typename, typename = int> class k {
public:
using promise_type = h;
@@ -64,6 +46,25 @@ my_coro (k<aj, ak> am, ai) {
;
}
+template <typename d> auto ab(int ac, d ad) -> decltype(ad.e(ac));
+int f;
+class h {
+ class j {
+ public:
+ bool await_ready() noexcept;
+ void await_suspend(std::coroutine_handle<>) noexcept;
+ void await_resume() noexcept;
+ };
+
+public:
+ k<int> get_return_object();
+ std::b initial_suspend();
+ j final_suspend() noexcept;
+ void unhandled_exception();
+ template <typename g>
+ auto await_transform (g c) { return ab(f, c); }
+};
+
void foo () {
k<int> a;
my_coro (a, [] {});
diff --git a/gcc/testsuite/g++.dg/coroutines/pr96749-2.C
b/gcc/testsuite/g++.dg/coroutines/pr96749-2.C
index 43052b57dd9..3d764527291 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr96749-2.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr96749-2.C
@@ -27,7 +27,7 @@ struct Task {
struct promise_type {
auto initial_suspend() { return std::suspend_always{}; }
auto final_suspend() noexcept { return std::suspend_always{}; }
- void get_return_object() {}
+ Task get_return_object() ;
void unhandled_exception() {}
};
};
