coroutines : Call promise CTOR with parm copies [PR97587].

Message ID B7EF0A8C-814B-4FCF-833A-72FE07A67B4F@sandoe.co.uk
State New
Headers show
Series
  • coroutines : Call promise CTOR with parm copies [PR97587].
Related show

Commit Message

Iain Sandoe Feb. 24, 2021, 8:09 p.m.
Hi,

As the PR notes, we were calling the promise CTOR with the original
function parameters, not the copy (as pointed, a previous wording of
the section on this was unambiguous).  Fixed thus.

tested on x86_64-darwin, x86_64-linux-gnu,
this is a wrong-code bug,

OK for master / 10.x?
thanks
Iain

gcc/cp/ChangeLog:

	PR c++/97587
	* coroutines.cc (struct param_info): Track rvalue refs.
	(morph_fn_to_coro): Track rvalue refs, and call the promise
	CTOR with the frame copy of passed parms.

gcc/testsuite/ChangeLog:

	PR c++/97587
	* g++.dg/coroutines/coro1-refs-and-ctors.h: Add a CTOR with two
	reference parms, to distinguish the rvalue ref. variant.
	* g++.dg/coroutines/pr97587.C: New test.
---
 gcc/cp/coroutines.cc                          | 25 ++++++++++-----
 .../g++.dg/coroutines/coro1-refs-and-ctors.h  |  7 ++--
 gcc/testsuite/g++.dg/coroutines/pr97587.C     | 32 +++++++++++++++++++
 3 files changed, 54 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr97587.C

-- 
2.24.1

Comments

Nathan Sidwell Feb. 25, 2021, 1:41 p.m. | #1
On 2/24/21 3:09 PM, Iain Sandoe wrote:
> Hi,

> 

> As the PR notes, we were calling the promise CTOR with the original

> function parameters, not the copy (as pointed, a previous wording of

> the section on this was unambiguous).  Fixed thus.

> 

> tested on x86_64-darwin, x86_64-linux-gnu,

> this is a wrong-code bug,

> 

> OK for master / 10.x?

> thanks

> Iain

> 

> gcc/cp/ChangeLog:

> 

> 	PR c++/97587

> 	* coroutines.cc (struct param_info): Track rvalue refs.

> 	(morph_fn_to_coro): Track rvalue refs, and call the promise

> 	CTOR with the frame copy of passed parms.

> 

> gcc/testsuite/ChangeLog:

> 

> 	PR c++/97587

> 	* g++.dg/coroutines/coro1-refs-and-ctors.h: Add a CTOR with two

> 	reference parms, to distinguish the rvalue ref. variant.

> 	* g++.dg/coroutines/pr97587.C: New test.


ok, thanks


-- 
Nathan Sidwell

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 19d2ca3e23e..0b63914ef9b 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1817,6 +1817,7 @@  struct param_info
   tree orig_type;    /* The original type of the parm (not as passed).  */
   bool by_ref;       /* Was passed by reference.  */
   bool pt_ref;       /* Was a pointer to object.  */
+  bool rv_ref;       /* Was an rvalue ref.  */
   bool trivial_dtor; /* The frame type has a trivial DTOR.  */
   bool this_ptr;     /* Is 'this' */
   bool lambda_cobj;  /* Lambda capture object */
@@ -4121,7 +4122,7 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	  if (actual_type == NULL_TREE)
 	    actual_type = error_mark_node;
 	  parm.orig_type = actual_type;
-	  parm.by_ref = parm.pt_ref = false;
+	  parm.by_ref = parm.pt_ref = parm.rv_ref =  false;
 	  if (TREE_CODE (actual_type) == REFERENCE_TYPE)
 	    {
 	      /* If the user passes by reference, then we will save the
@@ -4129,8 +4130,10 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 		 [dcl.fct.def.coroutine] / 13, if the lifetime of the
 		 referenced item ends and then the coroutine is resumed,
 		 we have UB; well, the user asked for it.  */
-	      actual_type = build_pointer_type (TREE_TYPE (actual_type));
-	      parm.pt_ref = true;
+	      if (TYPE_REF_IS_RVALUE (actual_type))
+		parm.rv_ref = true;
+	      else
+		parm.pt_ref = true;
 	    }
 	  else if (TYPE_REF_P (DECL_ARG_TYPE (arg)))
 	    parm.by_ref = true;
@@ -4498,16 +4501,22 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	      tree this_ref = build1 (INDIRECT_REF, ct, arg);
 	      tree rt = cp_build_reference_type (ct, false);
 	      this_ref = convert_to_reference (rt, this_ref, CONV_STATIC,
-					       LOOKUP_NORMAL , NULL_TREE,
+					       LOOKUP_NORMAL, NULL_TREE,
 					       tf_warning_or_error);
 	      vec_safe_push (promise_args, this_ref);
 	    }
-	  else if (parm.by_ref)
-	    vec_safe_push (promise_args, fld_idx);
+	  else if (parm.rv_ref)
+	    vec_safe_push (promise_args, rvalue(fld_idx));
 	  else
-	    vec_safe_push (promise_args, arg);
+	    vec_safe_push (promise_args, fld_idx);
 
-	  if (TYPE_NEEDS_CONSTRUCTING (parm.frame_type))
+	  if (parm.rv_ref || parm.pt_ref)
+	    /* Initialise the frame reference field directly.  */
+	    r = build_modify_expr (fn_start, TREE_OPERAND (fld_idx, 0),
+				   parm.frame_type, INIT_EXPR,
+				   DECL_SOURCE_LOCATION (arg), arg,
+				   DECL_ARG_TYPE (arg));
+	  else if (TYPE_NEEDS_CONSTRUCTING (parm.frame_type))
 	    {
 	      vec<tree, va_gc> *p_in;
 	      if (CLASS_TYPE_P (parm.frame_type)
diff --git a/gcc/testsuite/g++.dg/coroutines/coro1-refs-and-ctors.h b/gcc/testsuite/g++.dg/coroutines/coro1-refs-and-ctors.h
index 8831a07875e..dd45a0e1f81 100644
--- a/gcc/testsuite/g++.dg/coroutines/coro1-refs-and-ctors.h
+++ b/gcc/testsuite/g++.dg/coroutines/coro1-refs-and-ctors.h
@@ -3,10 +3,13 @@  struct coro1 {
   struct promise_type {
 
   promise_type () : vv(-1) {  PRINT ("Promise def. CTOR"); }
-  promise_type (int __x) : vv(__x) {  PRINTF ("Created Promise with %d\n",__x); }
+  promise_type (int __x) : vv(__x) {  PRINTF ("promise_type1 with %d\n",__x); }
   promise_type (int __x, int& __y, int&& __z)
     : vv(__x), v2(__y), v3(__z)
-    {  PRINTF ("Created Promise with %d, %d, %d\n", __x, __y, __z); }
+    {  PRINTF ("promise_type2 with %d, %d, %d\n", __x, __y, __z); }
+  promise_type (int __x, int& __y, int& __z)
+    : vv(__x), v2(__y), v3(__z)
+    {  PRINTF ("promise_type3 with %d, %d, %d\n", __x, __y, __z); }
 
   ~promise_type() { PRINT ("Destroyed Promise"); }
 
diff --git a/gcc/testsuite/g++.dg/coroutines/pr97587.C b/gcc/testsuite/g++.dg/coroutines/pr97587.C
new file mode 100644
index 00000000000..081c3a94b3c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr97587.C
@@ -0,0 +1,32 @@ 
+//  { dg-do run }
+
+#include<cassert>
+#include<coroutine>
+
+int *parameter_addr_in_promise_ctor;
+
+struct return_object{
+    struct promise_type{
+
+        promise_type(int &parameter)
+        {
+            parameter_addr_in_promise_ctor = &parameter;
+        }
+
+        return_object get_return_object(){ return {}; }
+
+        void return_void(){}
+
+        auto initial_suspend(){ return std::suspend_never{}; }
+        auto final_suspend() noexcept { return std::suspend_never{}; }
+        void unhandled_exception(){}
+    };
+};
+return_object coroutine(int parameter = 42){
+    assert(&parameter == parameter_addr_in_promise_ctor);
+    co_return;
+}
+
+int main(int,char**){
+    coroutine();
+}