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 <iain@sandoe.co.uk>
Co-authored-by: Jason Merrill <jason@redhat.com>
(cherry picked from commit 4c743798b1d4530b327dad7c606c610f3811fdbf)

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 09e2f34106277c057dbeebdcd7352655110c455e..8811d249c025aadb69b3336d100af155487f58bc 100644 (file)
--- 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 af9ef661a9360ffd9f13c40ccb61c4d2606effdf..7a14cb2eb892d87ea4934522a64eda4e594bf222 100644 (file)
--- 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 (file)
index 0000000..57a6e36
--- /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 (file)
index 0000000..f0db6a2
--- /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{};
+}