[Bug c++/100476] New: coroutines: questionable handling of void get_return_object

[Bug c++/100476] New: coroutines: questionable handling of void get_return_object

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

            Bug ID: 100476
           Summary: coroutines: questionable handling of void
                    get_return_object
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jason at gcc dot gnu.org
                CC: iains at gcc dot gnu.org
  Target Milestone: ---

morph_fn_to_coro includes the comment

      /* For class type return objects, we can attempt to construct,
         even if the gro is void.  */

I don't see anything in the standard to allow this.

Disabling that support breaks pr96749-2.C, pr94879-folly-1.C,
pr94883-folly-2.C, which all have void get_return_object() and a coroutine that
returns non-void.  I believe this is the result of testcase reduction, not
something that a user actually wrote.

Munging the second of those to appease clang gives the error I expect:

https://godbolt.org/z/PTbsWf8hj

<source>:50:3: error: no viable conversion from returned value of type 'void'
to function return type 'n'

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #1 from Iain Sandoe <iains at gcc dot gnu.org> ---
I am by no means an expert at reading standardese - and it might be that I'm
not alone, (library writers might have made the same assumption) but it seems
to me that:

http://eel.is/c++draft/dcl.fct.def.coroutine#7

Doe not make any specification about what get_­return_­object() returns, only
that it is possible to use it either (a) to initialise the return value
directly or (b) to construct a return value object from it.

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #2 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #1)
> I am by no means an expert at reading standardese - and it might be that I'm
> not alone, (library writers might have made the same assumption) but it
> seems to me that:
> 
> http://eel.is/c++draft/dcl.fct.def.coroutine#7
> 
> Doe not make any specification about what get_­return_­object() returns,
> only that it is possible to use it either (a) to initialise the return value
> directly or (b) to construct a return value object from it.

It says that you initialize the return value from get_return_object(), which
sounds like

  return promise.get_return_object();

in a non-coroutine, which is an error if get_return_object returns void.

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[jason at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #3 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #2)
> (In reply to Iain Sandoe from comment #1)
> > I am by no means an expert at reading standardese - and it might be that I'm
> > not alone, (library writers might have made the same assumption) but it
> > seems to me that:
> > 
> > http://eel.is/c++draft/dcl.fct.def.coroutine#7
> > 
> > Doe not make any specification about what get_­return_­object() returns,
> > only that it is possible to use it either (a) to initialise the return value
> > directly or (b) to construct a return value object from it.
> 
> It says that you initialize the return value from get_return_object(), which
> sounds like
> 
>   return promise.get_return_object();
> 
> in a non-coroutine, which is an error if get_return_object returns void.

In general, you can't construct an object from a void expression; a void
expression can't be used as an operand for anything.

struct A { };

A a1 = A(); // default-initialize
A a2 = A((void)0); // error

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #4 from Iain Sandoe <iains at gcc dot gnu.org> ---
great, that does simplify things - and means that a useful error can be
diagnosed from a mismatched type between g_r_o/g_r_o_o_a_f and the coroutine
ramp return value.

The statement about phasing of the calls does mean that the expression has to
be evaluated quite early (before the initial call to the actor function).

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #5 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #3)
> (In reply to Jason Merrill from comment #2)
> > (In reply to Iain Sandoe from comment #1)
> > > I am by no means an expert at reading standardese - and it might be that I'm
> > > not alone, (library writers might have made the same assumption) but it
> > > seems to me that:

> In general, you can't construct an object from a void expression; a void
> expression can't be used as an operand for anything.

well, we were not trying to do that - the erroneous behaviour was treating a
void return from g_r_o as meaning we should construct the return value like:

retval = CoroRetType();

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #6 from Iain Sandoe <iains at gcc dot gnu.org> ---
Looking at a current discussion of 
https://cplusplus.github.io/CWG/issues/2563.html

it seems that there is agreement that the returned type from
get_return_object() does not need to match the coroutine (ramp) return type -
so long at that latter type has a CTOR that accepts the get_return_object()
type.  Am I mistaken in thinking that void is a valid example of this?

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2024-06-29
           Assignee|unassigned at gcc dot gnu.org      |iains at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #7 from Iain Sandoe <iains at gcc dot gnu.org> ---
(In reply to Iain Sandoe from comment #6)
> Looking at a current discussion of 
> https://cplusplus.github.io/CWG/issues/2563.html
> 
> it seems that there is agreement that the returned type from
> get_return_object() does not need to match the coroutine (ramp) return type
> - so long at that latter type has a CTOR that accepts the
> get_return_object() type.  Am I mistaken in thinking that void is a valid
> example of this?

More discussion at WG21 - with the result that we do not intend to be able
handle a void get_return_object() - only ones that can be converted to the
return type.  So I'll look into revising the ramp to remove the extra code that
allows it.

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #8 from Iain Sandoe <iains at gcc dot gnu.org> ---
Created attachment 58946
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=58946&action=edit
patch under test

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

--- Comment #9 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain D Sandoe <iains@gcc.gnu.org>:

https://gcc.gnu.org/g:a0b431033c307982123abbff752045cfe7eda47f

commit r15-3151-ga0b431033c307982123abbff752045cfe7eda47f
Author: Iain Sandoe <iain@sandoe.co.uk>
Date:   Sat Aug 17 15:47:58 2024 +0100

    c++, coroutines: Only allow void get_return_object if the ramp is void
[PR100476].

    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>

[Bug c++/100476] coroutines: questionable handling of void get_return_object

[iains at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100476

Iain Sandoe <iains at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |WAITING

--- Comment #10 from Iain Sandoe <iains at gcc dot gnu.org> ---
fixed on trunk, waiting for possible back-port