| Message ID | 20240823183033.55719-1-iain@sandoe.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 8/23/24 2:30 PM, Iain Sandoe wrote: > Hi Jason > >>> + /* 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? > > Yes the idea is to try and test as much of the stuff that needs to be diagnosed > early, so that we do not need to tweak the input_pointer to get sensible messages. > However, in this case what you suggest will work fine - we can use error_at (....). > So now checking once we have the get_ro. > > retested on x86_64-darwin, OK for trunk? OK. > thanks > Iain > > --- 8< --- > > 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 <iain@sandoe.co.uk> > --- > gcc/cp/coroutines.cc | 47 +++++++++---------- > .../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(+), 49 deletions(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index 2f128db42e1..3c0757403ce 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -4605,6 +4605,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 > @@ -4771,7 +4772,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); > @@ -4971,17 +4972,27 @@ cp_coroutine_transform::build_ramp_function () > coro_get_return_object_identifier, > fn_start, NULL, /*musthave=*/true); > /* Without a return object we haven't got much clue what's going on. */ > - if (get_ro == error_mark_node) > + if (!get_ro || get_ro == error_mark_node) > { > BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_outer_bind); > /* Suppress warnings about the missing return value. */ > suppress_warning (orig_fn_decl, OPT_Wreturn_type); > return false; > } > + > + /* 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); > + if (VOID_TYPE_P (gro_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); > + return false; > + } > > 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; > @@ -4990,8 +5001,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 > @@ -5072,28 +5086,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); > - return false; > - } > > /* Without a relevant location, bad conversions in finish_return_stmt result > in unusable diagnostics, since there is not even a mention of the > 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() {} > }; > };
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 2f128db42e1..3c0757403ce 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -4605,6 +4605,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 @@ -4771,7 +4772,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); @@ -4971,17 +4972,27 @@ cp_coroutine_transform::build_ramp_function () coro_get_return_object_identifier, fn_start, NULL, /*musthave=*/true); /* Without a return object we haven't got much clue what's going on. */ - if (get_ro == error_mark_node) + if (!get_ro || get_ro == error_mark_node) { BIND_EXPR_BODY (ramp_bind) = pop_stmt_list (ramp_outer_bind); /* Suppress warnings about the missing return value. */ suppress_warning (orig_fn_decl, OPT_Wreturn_type); return false; } + + /* 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); + if (VOID_TYPE_P (gro_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); + return false; + } 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; @@ -4990,8 +5001,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 @@ -5072,28 +5086,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); - return false; - } /* Without a relevant location, bad conversions in finish_return_stmt result in unusable diagnostics, since there is not even a mention of the 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() {} }; };