> On 17 Sep 2024, at 20:06, Jason Merrill <ja...@redhat.com> wrote:
>
> On 9/17/24 8:26 PM, Iain Sandoe wrote:
>>> On 17 Sep 2024, at 18:24, Jason Merrill <ja...@redhat.com> wrote:
>>>
>>> On 8/29/24 5:22 PM, Iain Sandoe wrote:
>>>> Tested on x86_64-darwin/linux, powerpc64le linux, OK for trunk?
>>>> thanks,
>>>> Iain
>>>> --- >8 ---
>>>> Awaiters always need to have a coroutine state frame copy since
>>>> they persist across potential supensions. It simplifies the later
>>>> analysis considerably to assign these early which we do when
>>>> building co_await expressions.
>>>> The cleanups in r15-3146-g47dbd69b1, unfortunately elided some of
>>>> processing used to cater for cases where the var created from an
>>>> xvalue, or is a pointer/reference type.
>>>
>>> This seems closely related to CWG2472, which was resolved as NAD; it sounds
>>> to me like an xvalue should be cast to an lvalue referring to the same
>>> object, not that it should be used to initialize a new variable of class
>>> type. See below...
>> Ah … I am not sure how to do that cast (mechanically) - but that would also
>> do what is required.
>
> int&& f();
> reinterpret_cast<int&>(f()) // lvalue
> -or-
> int&& r = f(); // r is an lvalue
(I was meaning more the APIs to implement the same action inside the compiler).
>
>>>> Corrected thus.
>>>> PR c++/116506
>>>> gcc/cp/ChangeLog:
>>>> * coroutines.cc (build_co_await): Ensure that xvalues are
>>>> materialised. Handle references/pointer values in awaiter
>>>> access expressions.
>>>> gcc/testsuite/ChangeLog:
>>>> * g++.dg/coroutines/pr116506.C: New test.
>>>> Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
>>>> ---
>>>> gcc/cp/coroutines.cc | 39 ++++++++++------
>>>> gcc/testsuite/g++.dg/coroutines/pr116506.C | 53 ++++++++++++++++++++++
>>>> 2 files changed, 77 insertions(+), 15 deletions(-)
>>>> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr116506.C
>>>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>>>> index d4d74838eb4..d208ce429db 100644
>>>> --- a/gcc/cp/coroutines.cc
>>>> +++ b/gcc/cp/coroutines.cc
>>>> @@ -1130,13 +1130,12 @@ build_co_await (location_t loc, tree a,
>>>> suspend_point_kind suspend_kind,
>>>> if (o_type && !VOID_TYPE_P (o_type))
>>>> o_type = complete_type_or_else (o_type, o);
>>>> - if (!o_type)
>>>> + if (!o_type || o_type == error_mark_node)
>>>> return error_mark_node;
>>>> if (TREE_CODE (o_type) != RECORD_TYPE)
>>>> {
>>>> - error_at (loc, "awaitable type %qT is not a structure",
>>>> - o_type);
>>>> + error_at (loc, "awaitable type %qT is not a structure", o_type);
>>>> return error_mark_node;
>>>> }
>>>> @@ -1162,20 +1161,32 @@ build_co_await (location_t loc, tree a,
>>>> suspend_point_kind suspend_kind,
>>>> if (!glvalue_p (o))
>>>> o = get_target_expr (o, tf_warning_or_error);
>>>> - tree e_proxy = o;
>>>> - if (glvalue_p (o))
>>>> + /* We know that we need a coroutine state frame variable for the
>>>> awaiter,
>>>> + since it must persist across suspension; so where one is needed,
>>>> build
>>>> + it now. */
>>>> + if (INDIRECT_REF_P (o))
>>>> + o = TREE_OPERAND (o, 0);
>>>> + tree e_proxy = STRIP_NOPS (o);
>>>> + o_type = TREE_TYPE (e_proxy);
>>>
>>> So if you have an lvalue 'o' derived from a reference, this will change 'o'
>>> to be the reference, or if it's derived from a pointer, to the pointer.
>>> This seems like weird special-casing to me. If you need a frame variable,
>>> how about always creating a reference variable for glvalue o, regardless of
>>> how that glvalue was derived?
>>>
>>> Why would you need a variable for an xvalue, but not for an lvalue?
>> The intent is to ensure that awaiter instances are lvalues so that they are
>> correctly preserved across suspensions - the reason for calling them out
>> specifically (c.f. any other temporary) is that doing so makes the analysis
>> code simpler.
>
> My point is that being an lvalue is distinct from being a variable, except
> for prvalues that need to be materialized; an xvalue you can cast to an
> value, e.g.
>
> int&& f();
> reinterpret_cast<int&>(f()) // lvalue
>
> It's true that initializing a variable is a way to turn an xvalue into an
> lvalue without reinterpret_cast:
>
> int&& r = f(); // r is an lvalue
>
> but it's not clear to me that it's worth paying the space in the coroutine
> frame if we don't also need it for lvalue o.
>
> Why don't you need a variable to preserve o across suspensions if it's a call
> returning lvalue reference?
We always need a space for the awaiter, unless it is already a
variable/parameter (or part of one).
> I suspect that the simple case is not lvalue_p, but !TREE_SIDE_EFFECTS.
That is likely where I’m going wrong - we must not generate a variable for any
case that already has one (or a parm), but we must for any case that is a
temporary.
So, I should adjust the logic to use !TREE_SIDE_EFFECTS.
does the intention make sense now?
thanks
Iain
>
> Jason
