https://gcc.gnu.org/g:b69209936680dabbb7bbe08e71646a2c25ece0bf
commit r14-11740-gb69209936680dabbb7bbe08e71646a2c25ece0bf
Author: Iain Sandoe <iains....@gmail.com>
Date: Thu Oct 31 08:40:08 2024 +0000
c++/coroutines: Fix awaiter var creation [PR116506]
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
PR c++/116880
gcc/cp/ChangeLog:
* coroutines.cc (build_co_await): Ensure that xvalues are
materialised. Handle references/pointer values in awaiter
access expressions.
(is_stable_lvalue): New.
* decl.cc (cxx_maybe_build_cleanup): Handle null arg.
gcc/testsuite/ChangeLog:
* g++.dg/coroutines/pr116506.C: New test.
* g++.dg/coroutines/pr116880.C: New test.
Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
Co-authored-by: Jason Merrill <ja...@redhat.com>
(cherry picked from commit 4c743798b1d4530b327dad7c606c610f3811fdbf)
Diff:
---
gcc/cp/coroutines.cc | 106 +++++++++++++++--------------
gcc/cp/decl.cc | 2 +-
gcc/testsuite/g++.dg/coroutines/pr116506.C | 53 +++++++++++++++
gcc/testsuite/g++.dg/coroutines/pr116880.C | 36 ++++++++++
4 files changed, 144 insertions(+), 53 deletions(-)
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 09e2f3410627..8811d249c025 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1066,6 +1066,28 @@ build_template_co_await_expr (location_t kw, tree type,
tree expr, tree kind)
return aw_expr;
}
+/* Is EXPR an lvalue that will refer to the same object after a resume?
+
+ This is close to asking tree_invariant_p of its address, but that doesn't
+ distinguish temporaries from other variables. */
+
+static bool
+is_stable_lvalue (tree expr)
+{
+ if (TREE_SIDE_EFFECTS (expr))
+ return false;
+
+ for (; handled_component_p (expr);
+ expr = TREE_OPERAND (expr, 0))
+ {
+ if (TREE_CODE (expr) == ARRAY_REF
+ && !TREE_CONSTANT (TREE_OPERAND (expr, 1)))
+ return false;
+ }
+
+ return (TREE_CODE (expr) == PARM_DECL
+ || (VAR_P (expr) && !is_local_temp (expr)));
+}
/* This performs [expr.await] bullet 3.3 and validates the interface obtained.
It is also used to build the initial and final suspend points.
@@ -1128,7 +1150,7 @@ 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)
@@ -1155,56 +1177,35 @@ build_co_await (location_t loc, tree a,
suspend_point_kind suspend_kind,
if (!awrs_meth || awrs_meth == error_mark_node)
return error_mark_node;
- /* To complete the lookups, we need an instance of 'e' which is built from
- 'o' according to [expr.await] 3.4.
-
- If we need to materialize this as a temporary, then that will have to be
- 'promoted' to a coroutine frame var. However, if the awaitable is a
- user variable, parameter or comes from a scope outside this function,
- then we must use it directly - or we will see unnecessary copies.
-
- If o is a variable, find the underlying var. */
- tree e_proxy = STRIP_NOPS (o);
- if (INDIRECT_REF_P (e_proxy))
- e_proxy = TREE_OPERAND (e_proxy, 0);
- while (TREE_CODE (e_proxy) == COMPONENT_REF)
+ /* [expr.await]/3.3 If o would be a prvalue, the temporary
+ materialization conversion ([conv.rval]) is applied. */
+ if (!glvalue_p (o))
+ o = get_target_expr (o, tf_warning_or_error);
+
+ /* [expr.await]/3.4 e is an lvalue referring to the result of evaluating the
+ (possibly-converted) o.
+
+ So, either reuse an existing stable lvalue such as a variable or
+ COMPONENT_REF thereof, or create a new a coroutine state frame variable
+ for the awaiter, since it must persist across suspension. */
+ tree e_var = NULL_TREE;
+ tree e_proxy = o;
+ if (is_stable_lvalue (o))
+ o = NULL_TREE; /* Use the existing entity. */
+ else /* We need a non-temp var. */
{
- e_proxy = TREE_OPERAND (e_proxy, 0);
- if (INDIRECT_REF_P (e_proxy))
- e_proxy = TREE_OPERAND (e_proxy, 0);
- if (TREE_CODE (e_proxy) == CALL_EXPR)
+ tree p_type = TREE_TYPE (o);
+ tree o_a = o;
+ if (glvalue_p (o))
{
- /* We could have operator-> here too. */
- tree op = TREE_OPERAND (CALL_EXPR_FN (e_proxy), 0);
- if (DECL_OVERLOADED_OPERATOR_P (op)
- && DECL_OVERLOADED_OPERATOR_IS (op, COMPONENT_REF))
- {
- e_proxy = CALL_EXPR_ARG (e_proxy, 0);
- STRIP_NOPS (e_proxy);
- gcc_checking_assert (TREE_CODE (e_proxy) == ADDR_EXPR);
- e_proxy = TREE_OPERAND (e_proxy, 0);
- }
+ /* Build a reference variable for a non-stable lvalue o. */
+ p_type = cp_build_reference_type (p_type, xvalue_p (o));
+ o_a = build_address (o);
+ o_a = cp_fold_convert (p_type, o_a);
}
- STRIP_NOPS (e_proxy);
- }
-
- /* Only build a temporary if we need it. */
- STRIP_NOPS (e_proxy);
- if (TREE_CODE (e_proxy) == PARM_DECL
- || (VAR_P (e_proxy) && !is_local_temp (e_proxy)))
- {
- e_proxy = o;
- o = NULL_TREE; /* The var is already present. */
- }
- else
- {
- tree p_type = o_type;
- if (glvalue_p (o))
- p_type = cp_build_reference_type (p_type, !lvalue_p (o));
- e_proxy = get_awaitable_var (suspend_kind, p_type);
- o = cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o,
- tf_warning_or_error);
- e_proxy = convert_from_reference (e_proxy);
+ e_var = get_awaitable_var (suspend_kind, p_type);
+ o = cp_build_init_expr (loc, e_var, o_a);
+ e_proxy = convert_from_reference (e_var);
}
/* I suppose we could check that this is contextually convertible to bool.
*/
@@ -1270,7 +1271,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_var, tf_none))
{
if (CONVERT_EXPR_P (dummy))
dummy = TREE_OPERAND (dummy, 0);
@@ -1281,7 +1282,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(). */
@@ -1294,8 +1296,8 @@ 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);
+ if (e_var)
+ e_proxy = e_var;
tree awrs_type = TREE_TYPE (TREE_TYPE (awrs_func));
tree suspend_kind_cst = build_int_cst (integer_type_node,
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index af9ef661a936..7a14cb2eb892 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -19082,7 +19082,7 @@ cxx_maybe_build_cleanup (tree decl, tsubst_flags_t
complain)
/* Assume no cleanup is required. */
cleanup = NULL_TREE;
- if (error_operand_p (decl))
+ if (!decl || error_operand_p (decl))
return cleanup;
/* Handle "__attribute__((cleanup))". We run the cleanup function
diff --git a/gcc/testsuite/g++.dg/coroutines/pr116506.C
b/gcc/testsuite/g++.dg/coroutines/pr116506.C
new file mode 100644
index 000000000000..57a6e3605661
--- /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();
+}
diff --git a/gcc/testsuite/g++.dg/coroutines/pr116880.C
b/gcc/testsuite/g++.dg/coroutines/pr116880.C
new file mode 100644
index 000000000000..f0db6a260445
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr116880.C
@@ -0,0 +1,36 @@
+#include <coroutine>
+
+struct promise_type;
+using handle_type = std::coroutine_handle<promise_type>;
+
+struct Co {
+ handle_type handle;
+ using promise_type = ::promise_type;
+
+ explicit Co(handle_type handle) : handle(handle) {}
+
+ bool await_ready() { return false; }
+ std::coroutine_handle<> await_suspend(handle_type handle);
+ void await_resume() {}
+};
+
+struct Done {};
+
+struct promise_type {
+ Co get_return_object();
+
+ std::suspend_always initial_suspend() { return {}; };
+ std::suspend_always final_suspend() noexcept { return {}; };
+ void return_value(Done) {}
+ void return_value(Co&&);
+ void unhandled_exception() { throw; };
+ Co&& await_transform(Co&& co) { return static_cast<Co&&>(co); }
+};
+
+Co tryToRun();
+
+Co init()
+{
+ co_await tryToRun();
+ co_return Done{};
+}
