Hi Jason,

>>+     returned reference or prvalue result object ...
>>+     When we use a local to hold this, it is decltype(auto).  */
>>+  tree gro_type
>>+    = finish_decltype_type (get_ro, /*id_expression_or_member_access_p*/true,

>This should be false, not true; a call is not an id-expr or member access.

fixed.

>>     }
>> -  /* Initialize the resume_idx_var to 0, meaning "not started".  */
>>-  coro_build_and_push_artificial_var_with_dve
>>-    (loc, coro_resume_index_id, short_unsigned_type_node,  orig_fn_decl,
>>-     build_zero_cst (short_unsigned_type_node), deref_fp);

>Moving this initialization doesn't seem connected to the type of gro, or 
>mentioned above?

A fly-by tidy up.. removed.

>>-        promise_live = true;
>>+        promise_life++;
>>     }

>Please add a Promise copy constructor, whether deleted or defined so that 
>promise_life is accurate if it's called.

Done.

OK for trunk now?
thanks
Iain

--- 8<--

The revised wording for coroutines, uses decltype(auto) for the
type of the get return object, which preserves references. The
test is expected to fail, since it attempts to initialize the
return object from an object that has already been destroyed.

gcc/cp/ChangeLog:

        * coroutines.cc
        (cp_coroutine_transform::build_ramp_function): Use
        decltype(auto) to determine the type of the temporary
        get_return_object.

gcc/testsuite/ChangeLog:

        * g++.dg/coroutines/pr115908.C: Count promise construction
        and destruction.

Signed-off-by: Iain Sandoe <i...@sandoe.co.uk>
---
 gcc/cp/coroutines.cc                       | 22 ++++---
 gcc/testsuite/g++.dg/coroutines/pr115908.C | 74 +++++++++++++++-------
 2 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 6169a81cea5..bd51c31e615 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -5120,8 +5120,11 @@ cp_coroutine_transform::build_ramp_function ()
   /* Check for a bad get return object type.
      [dcl.fct.def.coroutine] / 7 requires:
      The expression promise.get_return_object() is used to initialize the
-     returned reference or prvalue result object ... */
-  tree gro_type = TREE_TYPE (get_ro);
+     returned reference or prvalue result object ...
+     When we use a local to hold this, it is decltype(auto).  */
+  tree gro_type
+    = finish_decltype_type (get_ro, /*id_expression_or_member_access_p*/false,
+                           tf_warning_or_error); // TREE_TYPE (get_ro);
   if (VOID_TYPE_P (gro_type) && !void_ramp_p)
     {
       error_at (fn_start, "no viable conversion from %<void%> provided by"
@@ -5129,11 +5132,6 @@ cp_coroutine_transform::build_ramp_function ()
       return false;
     }
 
-  /* Initialize the resume_idx_var to 0, meaning "not started".  */
-  coro_build_and_push_artificial_var_with_dve
-    (loc, coro_resume_index_id, short_unsigned_type_node,  orig_fn_decl,
-     build_zero_cst (short_unsigned_type_node), deref_fp);
-
   /* We must manage the cleanups ourselves, with the exception of the g_r_o,
      because the responsibility for them changes after the initial suspend.
      However, any use of cxx_maybe_build_cleanup () in preceding code can
@@ -5159,7 +5157,7 @@ cp_coroutine_transform::build_ramp_function ()
        = coro_build_and_push_artificial_var (loc, "_Coro_gro", gro_type,
                                              orig_fn_decl, NULL_TREE);
 
-      r = cp_build_init_expr (coro_gro, get_ro);
+      r = cp_build_init_expr (coro_gro, STRIP_REFERENCE_REF (get_ro));
       finish_expr_stmt (r);
       tree coro_gro_cleanup
        = cxx_maybe_build_cleanup (coro_gro, tf_warning_or_error);
@@ -5167,6 +5165,11 @@ cp_coroutine_transform::build_ramp_function ()
        push_cleanup (coro_gro, coro_gro_cleanup, /*eh_only*/false);
     }
 
+  /* Initialize the resume_idx_var to 0, meaning "not started".  */
+  coro_build_and_push_artificial_var_with_dve
+    (loc, coro_resume_index_id, short_unsigned_type_node,  orig_fn_decl,
+     build_zero_cst (short_unsigned_type_node), deref_fp);
+
   /* Start the coroutine body.  */
   r = build_call_expr_loc (fn_start, resumer, 1, coro_fp);
   finish_expr_stmt (r);
@@ -5180,7 +5183,8 @@ cp_coroutine_transform::build_ramp_function ()
   /* The ramp is done, we just need the return statement, which we build from
      the return object we constructed before we called the function body.  */
 
-  finish_return_stmt (void_ramp_p ? NULL_TREE : coro_gro);
+  r = void_ramp_p ? NULL_TREE : convert_from_reference (coro_gro);
+  finish_return_stmt (r);
 
   if (flag_exceptions)
     {
diff --git a/gcc/testsuite/g++.dg/coroutines/pr115908.C 
b/gcc/testsuite/g++.dg/coroutines/pr115908.C
index ac27d916de2..d8903748e40 100644
--- a/gcc/testsuite/g++.dg/coroutines/pr115908.C
+++ b/gcc/testsuite/g++.dg/coroutines/pr115908.C
@@ -6,23 +6,26 @@
 
 struct Promise;
 
-bool promise_live = false;
+int promise_life = 0;
 
 struct Handle : std::coroutine_handle<Promise> {
+
+    /* We now expect the handle to be created after the promise is destroyed.  
*/
     Handle(Promise &p) : 
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
-        if (!promise_live)
-          __builtin_abort ();
 #ifdef OUTPUT
-        std::cout << "Handle(Promise &)\n";
+        std::cout << "Handle(Promise &) " << promise_life << std::endl;
 #endif
-    }
-    Handle(Promise &&p) : 
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
-        if (!promise_live)
+         if (promise_life <= 0)
           __builtin_abort ();
+   }
+
+    Handle(Promise &&p) : 
std::coroutine_handle<Promise>(Handle::from_promise(p)) {
 #ifdef OUTPUT
-        std::cout << "Handle(Promise &&)\n";
+        std::cout << "Handle(Promise &&) "  << promise_life  << std::endl;
 #endif
-    }
+         if (promise_life <= 0)
+          __builtin_abort ();
+   }
 
     using promise_type = Promise;
 };
@@ -30,46 +33,73 @@ struct Handle : std::coroutine_handle<Promise> {
 struct Promise {
     Promise() {
 #ifdef OUTPUT
-        std::cout << "Promise()\n";
+        std::cout << "Promise()" << std::endl;
 #endif
-        promise_live = true;
+        promise_life++;
     }
+
+    Promise(Promise& p){
+#ifdef OUTPUT
+        std::cout << "Promise(Promise&)" << std::endl;
+#endif
+        promise_life++;
+    }
+
     ~Promise() {
 #ifdef OUTPUT
-        std::cout << "~Promise()\n";
+        std::cout << "~Promise()" << std::endl;
 #endif
-        if (!promise_live)
+        if (promise_life <= 0)
           __builtin_abort ();
-        promise_live = false;
+        promise_life--;
     }
+
     Promise& get_return_object() noexcept {
 #ifdef OUTPUT
-        std::cout << "get_return_object()\n";
+        std::cout << "get_return_object() " << promise_life << std::endl;
 #endif
-        if (!promise_live)
+        if (promise_life <= 0)
           __builtin_abort ();
         return *this;
     }
-    std::suspend_never initial_suspend() const noexcept { return {}; }
-    std::suspend_never final_suspend() const noexcept { return {}; }
+
+    std::suspend_never initial_suspend() const noexcept {
+#ifdef OUTPUT
+        std::cout << "initial_suspend()" << std::endl;
+#endif
+     return {}; 
+    }
+    std::suspend_never final_suspend() const noexcept {
+#ifdef OUTPUT
+        std::cout << "final_suspend()" << std::endl;
+#endif
+    return {};
+    }
     void return_void() const noexcept {
-        if (!promise_live)
+        if (!promise_life)
           __builtin_abort ();
 #ifdef OUTPUT
-        std::cout << "return_void()\n";
+        std::cout << "return_void()" << std::endl;
 #endif
     }
     void unhandled_exception() const noexcept {}
 };
 
 Handle Coro() {
+
+#ifdef OUTPUT
+        std::cout << "Coro()" << std::endl;
+#endif
     co_return;
 }
 
 int main() {
-  Coro();
 
-  if (promise_live)
+  Coro();
+#ifdef OUTPUT
+        std::cout << "done Coro()" << std::endl;
+#endif
+  if (promise_life)
     __builtin_abort ();
   return 0;
 }
-- 
2.39.2 (Apple Git-143)

Reply via email to