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
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? I suspect that the simple case is not
lvalue_p, but !TREE_SIDE_EFFECTS.
Jason
