Hi Jason,
gentle ping for this one
> On 29 Aug 2024, at 16:22, Iain Sandoe <iains....@gmail.com> 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.
>
> 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);
> + 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();
> +}
> --
> 2.39.2 (Apple Git-143)
>
