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...
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?
+ if (lvalue_p (e_proxy))
o = NULL_TREE; /* Use the existing entity. */
- else /* We need to materialise it. */
+ else /* We need a non-temp var. */
{
e_proxy = get_awaitable_var (suspend_kind, o_type);
o = cp_build_init_expr (loc, e_proxy, o);
- e_proxy = convert_from_reference (e_proxy);
}
+ /* Build an expression for the object that will be used to call the awaitable
+ methods. */
+ tree e_object = convert_from_reference (e_proxy);
+ if (POINTER_TYPE_P (TREE_TYPE (e_object)))
+ e_object = cp_build_indirect_ref (input_location, e_object, RO_UNARY_STAR,
+ tf_warning_or_error);
+
/* I suppose we could check that this is contextually convertible to bool.
*/
tree awrd_func = NULL_TREE;
tree awrd_call
- = build_new_method_call (e_proxy, awrd_meth, NULL, NULL_TREE,
LOOKUP_NORMAL,
+ = build_new_method_call (e_object, awrd_meth, NULL, NULL_TREE,
LOOKUP_NORMAL,
&awrd_func, tf_warning_or_error);
if (!awrd_func || !awrd_call || awrd_call == error_mark_node)
@@ -1189,7 +1200,7 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
tree h_proxy = get_coroutine_self_handle_proxy (current_function_decl);
vec<tree, va_gc> *args = make_tree_vector_single (h_proxy);
tree awsp_call
- = build_new_method_call (e_proxy, awsp_meth, &args, NULL_TREE,
+ = build_new_method_call (e_object, awsp_meth, &args, NULL_TREE,
LOOKUP_NORMAL, &awsp_func, tf_warning_or_error);
release_tree_vector (args);
@@ -1221,7 +1232,7 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
/* Finally, the type of e.await_resume() is the co_await's type. */
tree awrs_func = NULL_TREE;
tree awrs_call
- = build_new_method_call (e_proxy, awrs_meth, NULL, NULL_TREE,
LOOKUP_NORMAL,
+ = build_new_method_call (e_object, awrs_meth, NULL, NULL_TREE,
LOOKUP_NORMAL,
&awrs_func, tf_warning_or_error);
if (!awrs_func || !awrs_call || awrs_call == error_mark_node)
@@ -1235,7 +1246,7 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
return error_mark_node;
if (coro_diagnose_throwing_fn (awrs_func))
return error_mark_node;
- if (tree dummy = cxx_maybe_build_cleanup (e_proxy, tf_none))
+ if (tree dummy = cxx_maybe_build_cleanup (e_object, tf_none))
{
if (CONVERT_EXPR_P (dummy))
dummy = TREE_OPERAND (dummy, 0);
@@ -1246,7 +1257,8 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
}
/* We now have three call expressions, in terms of the promise, handle and
- 'e' proxies. Save them in the await expression for later expansion. */
+ 'e' proxy expression. Save them in the await expression for later
+ expansion. */
tree awaiter_calls = make_tree_vec (3);
TREE_VEC_ELT (awaiter_calls, 0) = awrd_call; /* await_ready(). */
@@ -1259,9 +1271,6 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
}
TREE_VEC_ELT (awaiter_calls, 2) = awrs_call; /* await_resume(). */
- if (REFERENCE_REF_P (e_proxy))
- e_proxy = TREE_OPERAND (e_proxy, 0);
-
tree awrs_type = TREE_TYPE (TREE_TYPE (awrs_func));
tree suspend_kind_cst = build_int_cst (integer_type_node,
(int) suspend_kind);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr116506.C
b/gcc/testsuite/g++.dg/coroutines/pr116506.C
new file mode 100644
index 00000000000..57a6e360566
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr116506.C
@@ -0,0 +1,53 @@
+// { dg-do run }
+// { dg-additional-options "-fno-exceptions" }
+
+#include <coroutine>
+
+bool g_too_early = true;
+std::coroutine_handle<> g_handle;
+
+struct Awaiter {
+ Awaiter() {}
+ ~Awaiter() {
+ if (g_too_early) {
+ __builtin_abort ();
+ }
+ }
+
+ bool await_ready() { return false; }
+ void await_suspend(std::coroutine_handle<> handle) {
+ g_handle = handle;
+ }
+
+ void await_resume() {}
+};
+
+struct SuspendNever {
+ bool await_ready() noexcept { return true; }
+ void await_suspend(std::coroutine_handle<>) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct Coroutine {
+ struct promise_type {
+ Coroutine get_return_object() { return {}; }
+ SuspendNever initial_suspend() { return {}; }
+ SuspendNever final_suspend() noexcept { return {}; }
+ void return_void() {}
+ void unhandled_exception() {}
+
+ Awaiter&& await_transform(Awaiter&& u) {
+ return static_cast<Awaiter&&>(u);
+ }
+ };
+};
+
+Coroutine foo() {
+ co_await Awaiter{};
+}
+
+int main() {
+ foo();
+ g_too_early = false;
+ g_handle.destroy();
+}
