coroutines: Pass class reference to promise param preview [PR94682]

Message ID 76533EE0-B942-430D-AD3C-95AEF2871955@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Pass class reference to promise param preview [PR94682]
Related show

Commit Message

Iain Sandoe April 21, 2020, 10:42 a.m.
Hi

As reported in the PR, per [dcl.fct.def.coroutine]/4 we should
be passing a reference to the object to the promise parameter
preview, and we are currently passing a pointer (this).

tested on x86_64-darwin, and in progress on x86_64/powerpc64-linux.
OK for master if regtesting is successful on Linux?
thanks
Iain

P.S. Probably:

	tree ct = TREE_TYPE (tt);
	tree rt = cp_build_reference_type (ct, false);
	tree this_ref = build_nop (rt, arg);

would be sufficient, but I’m a bit nervous about using short-circuits
after the experience with the ramp return.

------------

gcc/cp/ChangeLog:

2020-04-21  Iain Sandoe  <iain@sandoe.co.uk>

	* coroutines.cc (struct param_info): Add a field to
	note that the param is 'this'.
	(morph_fn_to_coro): Convert this to a reference before
	using it in the promise parameter preview.

gcc/testsuite/ChangeLog:

2020-04-21  Iain Sandoe  <iain@sandoe.co.uk>

	* g++.dg/coroutines/promise-parm-preview-this.C: New test.
---
 gcc/cp/coroutines.cc                          | 32 +++++++++++++------
 .../coroutines/promise-parm-preview-this.C    | 27 ++++++++++++++++
 2 files changed, 50 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C

-- 
2.24.1

Comments

Nathan Sidwell April 21, 2020, 1:01 p.m. | #1
On 4/21/20 6:42 AM, Iain Sandoe wrote:
> Hi

> 

> As reported in the PR, per [dcl.fct.def.coroutine]/4 we should

> be passing a reference to the object to the promise parameter

> preview, and we are currently passing a pointer (this).


> gcc/cp/ChangeLog:

> 

> 2020-04-21  Iain Sandoe  <iain@sandoe.co.uk>

> 

> 	* coroutines.cc (struct param_info): Add a field to

> 	note that the param is 'this'.

> 	(morph_fn_to_coro): Convert this to a reference before

> 	using it in the promise parameter preview.


ok, one nit

> diff --git a/gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C b/gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C


> +  future foo(int param) { co_return 0; }

> +};

> \ No newline at end of file


^ needs a new line

nathan

-- 
Nathan Sidwell

Patch

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 30676eba6c2..b1d91f84cae 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -1760,14 +1760,15 @@  transform_await_wrapper (tree *stmt, int *do_subtree, void *d)
 
 struct param_info
 {
-  tree field_id;  /* The name of the copy in the coroutine frame.  */
+  tree field_id;     /* The name of the copy in the coroutine frame.  */
   vec<tree *> *body_uses; /* Worklist of uses, void if there are none.  */
-  tree frame_type; /* The type used to represent this parm in the frame.  */
-  tree orig_type;  /* The original type of the parm (not as passed).  */
-  bool by_ref;  /* Was passed by reference.  */
-  bool rv_ref;  /* Was an rvalue reference.  */
-  bool pt_ref;  /* Was a pointer to object.  */
+  tree frame_type;   /* The type used to represent this parm in the frame.  */
+  tree orig_type;    /* The original type of the parm (not as passed).  */
+  bool by_ref;       /* Was passed by reference.  */
+  bool rv_ref;       /* Was an rvalue reference.  */
+  bool pt_ref;       /* Was a pointer to object.  */
   bool trivial_dtor; /* The frame type has a trivial DTOR.  */
+  bool this_ptr;     /* Is 'this' */
 };
 
 struct local_var_info
@@ -3279,7 +3280,7 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	    }
 	  else
 	    parm.frame_type = actual_type;
-
+	  parm.this_ptr = is_this_parameter (arg);
 	  parm.trivial_dtor = TYPE_HAS_TRIVIAL_DESTRUCTOR (parm.frame_type);
 	  tree pname = DECL_NAME (arg);
 	  char *buf = xasprintf ("__parm.%s", IDENTIFIER_POINTER (pname));
@@ -3617,8 +3618,21 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 					      false, tf_warning_or_error);
 
 	  /* Add this to the promise CTOR arguments list, accounting for
-	     refs.  */
-	  if (parm.by_ref)
+	     refs and this ptr.  */
+	  if (parm.this_ptr)
+	    {
+	      /* We pass a reference to *this to the param preview.  */
+	      tree tt = TREE_TYPE (arg);
+	      gcc_checking_assert (POINTER_TYPE_P (tt));
+	      tree ct = TREE_TYPE (tt);
+	      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,
+					       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));
diff --git a/gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C b/gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C
new file mode 100644
index 00000000000..ca96f373da2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/promise-parm-preview-this.C
@@ -0,0 +1,27 @@ 
+#include "coro.h"
+
+struct promise;
+
+struct future
+{
+    using promise_type = promise;
+};
+
+struct promise
+{
+  template<typename Class>
+  promise(Class &,int) { static_assert(!std::is_pointer<Class>::value, ""); }
+
+  coro::suspend_never initial_suspend() { return {}; }
+  coro::suspend_never final_suspend() { return {}; }
+
+  future get_return_object() { return {}; }
+
+  void return_value(int) {}
+  void unhandled_exception() {}
+};
+
+struct bar
+{
+  future foo(int param) { co_return 0; }
+};
\ No newline at end of file