coroutines: Pass class ref to traits lookup and promise allocator [PR94760]

Message ID FC9FAA1A-0566-4DFE-9FC5-D326DEF867F2@sandoe.co.uk
State New
Headers show
Series
  • coroutines: Pass class ref to traits lookup and promise allocator [PR94760]
Related show

Commit Message

Iain Sandoe April 25, 2020, 3:08 p.m.
(WAS  [PATCH] coroutines: Handle lambda capture objects in the way as clang.)

I am sorry to mess you around on this.

You had approved a previous patch, which I then withdrew because of the confusion
about what each implementation was doing (you were on that long email thread).

Now that MSVC and clang folks, have clarified things + we have a testcase that demonstrates
rejecting valid code .. I’d like to ask for the following updated patch to be approved, and there’s
a PR tracking the rejects-valid.

Tested locally,
OK after re-resting on x86-64-linux?
thanks
Iain


This will be followed up with one implementing the change for lambda object pointers.

Iain Sandoe <iain@sandoe.co.uk> wrote:

> Iain Sandoe <iain@sandoe.co.uk> wrote:

> 

>> Nathan Sidwell <nathan@acm.org> wrote:

>> 

>>> On 4/22/20 8:48 AM, Iain Sandoe wrote:

>>>> Hi,

>>>> There is no PR for this, at present, but the implementation of

>>>> clang and GCC's handling of lambda capture object implicit parms

>>>> is currently different.  There is still some discussion about

>>>> 'correct' interpretation of the standard - but in the short-term

>>>> it is probably best to have consistent implementations - even if

>>>> those subsequently turn out to be 'consistently wrong'.

>>> 

>>> Agreed, the std is at best ambigiuous in this area, we should aim for implementation agreement.

>> 

>> following more discussion amongst WG21 members, it appears that there is still

>> some confusion over the status of other implementations, and it may well be that

>> clang will be updated to follow the pattern that GCC is currently implementing.

>> 

>> In light of this, perhaps it’s best to withdraw this patch for now.

> 

> I think we should apply the following, pending the resolution of the ‘correct’

> action for the lambda closure object.  This brings GCC into line with clang for the

> handing of ‘this’ in method coroutines, but leaves it untouched for lambda closure

> object pointers.


-- 
2.24.1

Comments

Nathan Sidwell April 27, 2020, 2 p.m. | #1
On 4/25/20 11:08 AM, Iain Sandoe wrote:
> (WAS  [PATCH] coroutines: Handle lambda capture objects in the way as clang.)

> 

> I am sorry to mess you around on this.


It wasn't you who wasn't focussing :)

> We changed the argument passed to the promise parameter preview

> to match a reference to *this.  However to be consistent with the

> other ports, we do need to match the reference transformation in

> the traits lookup and the promise allocator lookup.


A few comments ...

> 

> gcc/cp/ChangeLog:

> 

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

> 

> 	PR c++/94760

> 	* coroutines.cc (instantiate_coro_traits): Pass a reference to

> 	object type rather than a pointer type for 'this', for method

> 	coroutines.

> 	(struct param_info): Add a field to hold that the parm is a lambda

> 	closure pointer.

> 	(morph_fn_to_coro): Check for lambda closure pointers in the

> 	args.  Use a reference to *this when building the args list for the

> 	promise allocator lookup.

> 

> gcc/testsuite/ChangeLog:

> 

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

> 

> 	PR c++/94760

> 	* g++.dg/coroutines/pr94760-mismatched-traits-and-promise-prev.C: New test.

> ---

>   gcc/cp/coroutines.cc                          | 52 +++++++++++++++++--

>   ...9xxxx-mismatched-traits-and-promise-prev.C | 29 +++++++++++

>   2 files changed, 77 insertions(+), 4 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C

> 

> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> index 4f254b7fd10..c80a3b716b2 100644

> --- a/gcc/cp/coroutines.cc

> +++ b/gcc/cp/coroutines.cc

> @@ -296,14 +296,25 @@ instantiate_coro_traits (tree fndecl, location_t kw)

>        type.  */

>   

>     tree functyp = TREE_TYPE (fndecl);

> +  tree arg = DECL_ARGUMENTS (fndecl);

> +  bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (fndecl));


I think LAMBDA_FUNCTION_P (fndecl) expresses intent better.

> @@ -3652,6 +3664,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   	  The second two entries start out empty - and only get populated

>   	  when we see uses.  */

>         param_uses = new hash_map<tree, param_info>;

> +      bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (orig));


Likewise.

>   

>         for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;

>   	   arg = DECL_CHAIN (arg))

> @@ -3691,7 +3704,17 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   	    }

>   	  else

>   	    parm.frame_type = actual_type;

> +

>   	  parm.this_ptr = is_this_parameter (arg);

> +	  if (lambda_p)

> +	    {

> +	      parm.lambda_cobj = parm.this_ptr

> +				 || (DECL_NAME (arg) == closure_identifier);


i'm confused by the need for || here.  Why isn't just the DECL_NAME test 
needed?  The parser appears to give the object parameter that name.  (If 
you do need the parm.this_ptr piece, it suggests somewhere else outside 
of coro is not being consistent, but ICBW.)

> @@ -3831,9 +3854,28 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   	those of the original function.  */

>         vec<tree, va_gc> *args = make_tree_vector ();

>         vec_safe_push (args, resizeable); /* Space needed.  */

> +

>         for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;

>   	   arg = DECL_CHAIN (arg))

> -	vec_safe_push (args, arg);

> +	{

> +	  param_info *parm_i = param_uses->get (arg);

> +	  gcc_checking_assert (parm_i);

> +	  if (parm_i->lambda_cobj)

> +	    vec_safe_push (args, arg);

> +	  else if (0 && parm_i->this_ptr)


^^ looks like now-disabled experimental code?


-- 
Nathan Sidwell
Iain Sandoe April 27, 2020, 6:41 p.m. | #2
Nathan Sidwell <nathan@acm.org> wrote:

> On 4/25/20 11:08 AM, Iain Sandoe wrote:


>> +  tree arg = DECL_ARGUMENTS (fndecl);

>> +  bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (fndecl));

> 

> I think LAMBDA_FUNCTION_P (fndecl) expresses intent better.

done in both places.

>> +

>>  	  parm.this_ptr = is_this_parameter (arg);

>> +	  if (lambda_p)

>> +	    {

>> +	      parm.lambda_cobj = parm.this_ptr

>> +				 || (DECL_NAME (arg) == closure_identifier);

> 

> i'm confused by the need for || here.  Why isn't just the DECL_NAME test needed?  The parser appears to give the object parameter that name.  (If you do need the parm.this_ptr piece, it suggests somewhere else outside of coro is not being consistent, but ICBW.)


It seems that, when a lambda is part of a class, the closure object gets the name ‘this’.
When a lambda is defined outside a class context, the closure object is named __closure.

I added a comment as to why we make the two checks (also, why we can’t just use
is_this_parameter() as the test elsewhere)

>> +	    vec_safe_push (args, arg);

>> +	  else if (0 && parm_i->this_ptr)

> 

> ^^ looks like now-disabled experimental code?

well caught, now enabled as intended.

thanks for the review is the revised below OK (testing now)

thanks
iain

===

We changed the argument passed to the promise parameter preview
to match a reference to *this.  However to be consistent with the
other ports, we do need to match the reference transformation in
the traits lookup and the promise allocator lookup.

gcc/cp/ChangeLog:

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

	PR c++/94760
	* coroutines.cc (instantiate_coro_traits): Pass a reference to
	object type rather than a pointer type for 'this', for method
	coroutines.
	(struct param_info): Add a field to hold that the parm is a lambda
	closure pointer.
	(morph_fn_to_coro): Check for lambda closure pointers in the
	args.  Use a reference to *this when building the args list for the
	promise allocator lookup.

gcc/testsuite/ChangeLog:

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

	PR c++/94760
	* g++.dg/coroutines/pr94760-mismatched-traits-and-promise-prev.C: New test.
---
 gcc/cp/coroutines.cc                          | 54 +++++++++++++++++--
 ...9xxxx-mismatched-traits-and-promise-prev.C | 29 ++++++++++
 2 files changed, 79 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 0a5a0c9b1d2..cfaa018c551 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -296,14 +296,25 @@ instantiate_coro_traits (tree fndecl, location_t kw)
      type.  */
 
   tree functyp = TREE_TYPE (fndecl);
+  tree arg = DECL_ARGUMENTS (fndecl);
+  bool lambda_p = LAMBDA_FUNCTION_P (fndecl);
   tree arg_node = TYPE_ARG_TYPES (functyp);
   tree argtypes = make_tree_vec (list_length (arg_node)-1);
   unsigned p = 0;
 
   while (arg_node != NULL_TREE && !VOID_TYPE_P (TREE_VALUE (arg_node)))
     {
-      TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);
+      if (is_this_parameter (arg) && !lambda_p)
+	{
+	  /* We pass a reference to *this to the param preview.  */
+	  tree ct = TREE_TYPE (TREE_TYPE (arg));
+	  TREE_VEC_ELT (argtypes, p++) = cp_build_reference_type (ct, false);
+	}
+      else
+	TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);
+
       arg_node = TREE_CHAIN (arg_node);
+      arg = DECL_CHAIN (arg);
     }
 
   tree argtypepack = cxx_make_type (TYPE_ARGUMENT_PACK);
@@ -1766,6 +1777,7 @@ struct param_info
   bool pt_ref;       /* Was a pointer to object.  */
   bool trivial_dtor; /* The frame type has a trivial DTOR.  */
   bool this_ptr;     /* Is 'this' */
+  bool lambda_cobj;  /* Lambda capture object */
 };
 
 struct local_var_info
@@ -3652,6 +3664,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	  The second two entries start out empty - and only get populated
 	  when we see uses.  */
       param_uses = new hash_map<tree, param_info>;
+      bool lambda_p = LAMBDA_FUNCTION_P (orig);
 
       unsigned no_name_parm = 0;
       for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
@@ -3692,7 +3705,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	    }
 	  else
 	    parm.frame_type = actual_type;
+
+	  /* The closure object pointer is called 'this' when a lambda is
+	     part of a class, and __closure when it is not.  */
 	  parm.this_ptr = is_this_parameter (arg);
+	  if (lambda_p)
+	    {
+	      parm.lambda_cobj = parm.this_ptr
+				 || (DECL_NAME (arg) == closure_identifier);
+	      parm.this_ptr = false;
+	    }
+	  else
+	    parm.lambda_cobj = false;
+
 	  parm.trivial_dtor = TYPE_HAS_TRIVIAL_DESTRUCTOR (parm.frame_type);
 	  char *buf;
 	  if (DECL_NAME (arg))
@@ -3838,9 +3863,28 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	those of the original function.  */
       vec<tree, va_gc> *args = make_tree_vector ();
       vec_safe_push (args, resizeable); /* Space needed.  */
+
       for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
 	   arg = DECL_CHAIN (arg))
-	vec_safe_push (args, arg);
+	{
+	  param_info *parm_i = param_uses->get (arg);
+	  gcc_checking_assert (parm_i);
+	  if (parm_i->lambda_cobj)
+	    vec_safe_push (args, arg);
+	  else if (parm_i->this_ptr)
+	    {
+	      /* We pass a reference to *this to the allocator lookup.  */
+	      tree tt = TREE_TYPE (TREE_TYPE (arg));
+	      tree this_ref = build1 (INDIRECT_REF, tt, arg);
+	      tt = cp_build_reference_type (tt, false);
+	      this_ref = convert_to_reference (tt, this_ref, CONV_STATIC,
+					       LOOKUP_NORMAL , NULL_TREE,
+					       tf_warning_or_error);
+	      vec_safe_push (args, this_ref);
+	    }
+	  else
+	    vec_safe_push (args, arg);
+	}
 
       /* We might need to check that the provided function is nothrow.  */
       tree func;
@@ -4036,8 +4080,10 @@ 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 and this ptr.  */
-	  if (parm.this_ptr)
+	     refs and special handling for method this ptr.  */
+	  if (parm.lambda_cobj)
+	    vec_safe_push (promise_args, arg);
+	  else if (parm.this_ptr)
 	    {
 	      /* We pass a reference to *this to the param preview.  */
 	      tree tt = TREE_TYPE (arg);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C b/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C
new file mode 100644
index 00000000000..235b5e757be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C
@@ -0,0 +1,29 @@
+// { dg-do compile  { target c++17 } }
+
+#include "coro.h"
+
+// Test that we get matching types to traits and promise param
+// preview.
+
+// A separate issue from allowing non-class return types.
+struct Fake {} ;
+
+template<typename R, typename CallOp, typename ...T>
+struct std::coroutine_traits<R, CallOp, T...> {
+    struct promise_type {
+        promise_type (CallOp op, T ...args) {}
+        Fake get_return_object() { return {}; }
+        std::suspend_always initial_suspend() { return {}; }
+        std::suspend_never final_suspend() { return {}; }
+        void return_void() {}
+        void unhandled_exception() {}
+    };
+};
+
+
+struct Foo
+{
+  Fake operator() (int a) {
+    co_return;
+  }
+};
-- 
2.24.1
Nathan Sidwell April 27, 2020, 8:19 p.m. | #3
On 4/27/20 2:41 PM, Iain Sandoe wrote:
> Nathan Sidwell <nathan@acm.org> wrote:

> 

>> On 4/25/20 11:08 AM, Iain Sandoe wrote:

> 

>>> +  tree arg = DECL_ARGUMENTS (fndecl);

>>> +  bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (fndecl));

>>

>> I think LAMBDA_FUNCTION_P (fndecl) expresses intent better.

> done in both places.

> 

>>> +

>>>   	  parm.this_ptr = is_this_parameter (arg);

>>> +	  if (lambda_p)

>>> +	    {

>>> +	      parm.lambda_cobj = parm.this_ptr

>>> +				 || (DECL_NAME (arg) == closure_identifier);

>>

>> i'm confused by the need for || here.  Why isn't just the DECL_NAME test needed?  The parser appears to give the object parameter that name.  (If you do need the parm.this_ptr piece, it suggests somewhere else outside of coro is not being consistent, but ICBW.)

> 

> It seems that, when a lambda is part of a class, the closure object gets the name ‘this’.

> When a lambda is defined outside a class context, the closure object is named __closure.

> 

> I added a comment as to why we make the two checks (also, why we can’t just use

> is_this_parameter() as the test elsewhere)

> 

>>> +	    vec_safe_push (args, arg);

>>> +	  else if (0 && parm_i->this_ptr)

>>

>> ^^ looks like now-disabled experimental code?

> well caught, now enabled as intended.

> 

> thanks for the review is the revised below OK (testing now)

> 

> thanks

> iain

> 

> ===

> 

> We changed the argument passed to the promise parameter preview

> to match a reference to *this.  However to be consistent with the

> other ports, we do need to match the reference transformation in

> the traits lookup and the promise allocator lookup.

> 

> gcc/cp/ChangeLog:

> 

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

> 

> 	PR c++/94760

> 	* coroutines.cc (instantiate_coro_traits): Pass a reference to

> 	object type rather than a pointer type for 'this', for method

> 	coroutines.

> 	(struct param_info): Add a field to hold that the parm is a lambda

> 	closure pointer.

> 	(morph_fn_to_coro): Check for lambda closure pointers in the

> 	args.  Use a reference to *this when building the args list for the

> 	promise allocator lookup.

> 

> gcc/testsuite/ChangeLog:

> 

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

> 

> 	PR c++/94760

> 	* g++.dg/coroutines/pr94760-mismatched-traits-and-promise-prev.C: New test.

> ---

>   gcc/cp/coroutines.cc                          | 54 +++++++++++++++++--

>   ...9xxxx-mismatched-traits-and-promise-prev.C | 29 ++++++++++

>   2 files changed, 79 insertions(+), 4 deletions(-)

>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C

> 

> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc

> index 0a5a0c9b1d2..cfaa018c551 100644

> --- a/gcc/cp/coroutines.cc

> +++ b/gcc/cp/coroutines.cc

> @@ -296,14 +296,25 @@ instantiate_coro_traits (tree fndecl, location_t kw)

>        type.  */

>   

>     tree functyp = TREE_TYPE (fndecl);

> +  tree arg = DECL_ARGUMENTS (fndecl);

> +  bool lambda_p = LAMBDA_FUNCTION_P (fndecl);

>     tree arg_node = TYPE_ARG_TYPES (functyp);

>     tree argtypes = make_tree_vec (list_length (arg_node)-1);

>     unsigned p = 0;

>   

>     while (arg_node != NULL_TREE && !VOID_TYPE_P (TREE_VALUE (arg_node)))

>       {

> -      TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);

> +      if (is_this_parameter (arg) && !lambda_p)

> +	{

> +	  /* We pass a reference to *this to the param preview.  */

> +	  tree ct = TREE_TYPE (TREE_TYPE (arg));

> +	  TREE_VEC_ELT (argtypes, p++) = cp_build_reference_type (ct, false);

> +	}

> +      else

> +	TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);

> +

>         arg_node = TREE_CHAIN (arg_node);

> +      arg = DECL_CHAIN (arg);

>       }

>   

>     tree argtypepack = cxx_make_type (TYPE_ARGUMENT_PACK);

> @@ -1766,6 +1777,7 @@ struct param_info

>     bool pt_ref;       /* Was a pointer to object.  */

>     bool trivial_dtor; /* The frame type has a trivial DTOR.  */

>     bool this_ptr;     /* Is 'this' */

> +  bool lambda_cobj;  /* Lambda capture object */

>   };

>   

>   struct local_var_info

> @@ -3652,6 +3664,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   	  The second two entries start out empty - and only get populated

>   	  when we see uses.  */

>         param_uses = new hash_map<tree, param_info>;

> +      bool lambda_p = LAMBDA_FUNCTION_P (orig);

>   

>         unsigned no_name_parm = 0;

>         for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;

> @@ -3692,7 +3705,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)

>   	    }

>   	  else

>   	    parm.frame_type = actual_type;

> +

> +	  /* The closure object pointer is called 'this' when a lambda is

> +	     part of a class, and __closure when it is not.  */


It turns out this occurs when tsubsting a lambda.  I.e. whenever the 
lambda is inside a templated function (real or pseudo).  IMHO that;s a 
bug in the template machinery, but let's not fix that right now, I have 
filed 94807.  Just update the comment to describe this as occurring 
during instantiation.

your patch is ok otherwise, thanks.

nathan

-- 
Nathan Sidwell

Patch

====

We changed the argument passed to the promise parameter preview
to match a reference to *this.  However to be consistent with the
other ports, we do need to match the reference transformation in
the traits lookup and the promise allocator lookup.

gcc/cp/ChangeLog:

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

	PR c++/94760
	* coroutines.cc (instantiate_coro_traits): Pass a reference to
	object type rather than a pointer type for 'this', for method
	coroutines.
	(struct param_info): Add a field to hold that the parm is a lambda
	closure pointer.
	(morph_fn_to_coro): Check for lambda closure pointers in the
	args.  Use a reference to *this when building the args list for the
	promise allocator lookup.

gcc/testsuite/ChangeLog:

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

	PR c++/94760
	* g++.dg/coroutines/pr94760-mismatched-traits-and-promise-prev.C: New test.
---
 gcc/cp/coroutines.cc                          | 52 +++++++++++++++++--
 ...9xxxx-mismatched-traits-and-promise-prev.C | 29 +++++++++++
 2 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C

diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
index 4f254b7fd10..c80a3b716b2 100644
--- a/gcc/cp/coroutines.cc
+++ b/gcc/cp/coroutines.cc
@@ -296,14 +296,25 @@  instantiate_coro_traits (tree fndecl, location_t kw)
      type.  */
 
   tree functyp = TREE_TYPE (fndecl);
+  tree arg = DECL_ARGUMENTS (fndecl);
+  bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (fndecl));
   tree arg_node = TYPE_ARG_TYPES (functyp);
   tree argtypes = make_tree_vec (list_length (arg_node)-1);
   unsigned p = 0;
 
   while (arg_node != NULL_TREE && !VOID_TYPE_P (TREE_VALUE (arg_node)))
     {
-      TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);
+      if (is_this_parameter (arg) && !lambda_p)
+	{
+	  /* We pass a reference to *this to the param preview.  */
+	  tree ct = TREE_TYPE (TREE_TYPE (arg));
+	  TREE_VEC_ELT (argtypes, p++) = cp_build_reference_type (ct, false);
+	}
+      else
+	TREE_VEC_ELT (argtypes, p++) = TREE_VALUE (arg_node);
+
       arg_node = TREE_CHAIN (arg_node);
+      arg = DECL_CHAIN (arg);
     }
 
   tree argtypepack = cxx_make_type (TYPE_ARGUMENT_PACK);
@@ -1766,6 +1777,7 @@  struct param_info
   bool pt_ref;       /* Was a pointer to object.  */
   bool trivial_dtor; /* The frame type has a trivial DTOR.  */
   bool this_ptr;     /* Is 'this' */
+  bool lambda_cobj;  /* Lambda capture object */
 };
 
 struct local_var_info
@@ -3652,6 +3664,7 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	  The second two entries start out empty - and only get populated
 	  when we see uses.  */
       param_uses = new hash_map<tree, param_info>;
+      bool lambda_p = LAMBDA_TYPE_P (DECL_CONTEXT (orig));
 
       for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
 	   arg = DECL_CHAIN (arg))
@@ -3691,7 +3704,17 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	    }
 	  else
 	    parm.frame_type = actual_type;
+
 	  parm.this_ptr = is_this_parameter (arg);
+	  if (lambda_p)
+	    {
+	      parm.lambda_cobj = parm.this_ptr
+				 || (DECL_NAME (arg) == closure_identifier);
+	      parm.this_ptr = false;
+	    }
+	  else
+	    parm.lambda_cobj = false;
+
 	  parm.trivial_dtor = TYPE_HAS_TRIVIAL_DESTRUCTOR (parm.frame_type);
 	  tree pname = DECL_NAME (arg);
 	  char *buf = xasprintf ("__parm.%s", IDENTIFIER_POINTER (pname));
@@ -3831,9 +3854,28 @@  morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
 	those of the original function.  */
       vec<tree, va_gc> *args = make_tree_vector ();
       vec_safe_push (args, resizeable); /* Space needed.  */
+
       for (tree arg = DECL_ARGUMENTS (orig); arg != NULL;
 	   arg = DECL_CHAIN (arg))
-	vec_safe_push (args, arg);
+	{
+	  param_info *parm_i = param_uses->get (arg);
+	  gcc_checking_assert (parm_i);
+	  if (parm_i->lambda_cobj)
+	    vec_safe_push (args, arg);
+	  else if (0 && parm_i->this_ptr)
+	    {
+	      /* We pass a reference to *this to the allocator lookup.  */
+	      tree tt = TREE_TYPE (TREE_TYPE (arg));
+	      tree this_ref = build1 (INDIRECT_REF, tt, arg);
+	      tt = cp_build_reference_type (tt, false);
+	      this_ref = convert_to_reference (tt, this_ref, CONV_STATIC,
+					       LOOKUP_NORMAL , NULL_TREE,
+					       tf_warning_or_error);
+	      vec_safe_push (args, this_ref);
+	    }
+	  else
+	    vec_safe_push (args, arg);
+	}
 
       /* We might need to check that the provided function is nothrow.  */
       tree func;
@@ -4029,8 +4071,10 @@  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 and this ptr.  */
-	  if (parm.this_ptr)
+	     refs and special handling for method this ptr.  */
+	  if (parm.lambda_cobj)
+	    vec_safe_push (promise_args, arg);
+	  else if (parm.this_ptr)
 	    {
 	      /* We pass a reference to *this to the param preview.  */
 	      tree tt = TREE_TYPE (arg);
diff --git a/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C b/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C
new file mode 100644
index 00000000000..235b5e757be
--- /dev/null
+++ b/gcc/testsuite/g++.dg/coroutines/pr9xxxx-mismatched-traits-and-promise-prev.C
@@ -0,0 +1,29 @@ 
+// { dg-do compile  { target c++17 } }
+
+#include "coro.h"
+
+// Test that we get matching types to traits and promise param
+// preview.
+
+// A separate issue from allowing non-class return types.
+struct Fake {} ;
+
+template<typename R, typename CallOp, typename ...T>
+struct std::coroutine_traits<R, CallOp, T...> {
+    struct promise_type {
+        promise_type (CallOp op, T ...args) {}
+        Fake get_return_object() { return {}; }
+        std::suspend_always initial_suspend() { return {}; }
+        std::suspend_never final_suspend() { return {}; }
+        void return_void() {}
+        void unhandled_exception() {}
+    };
+};
+
+
+struct Foo
+{
+  Fake operator() (int a) {
+    co_return;
+  }
+};