diff mbox series

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] | expand

Commit Message

Iain Sandoe April 25, 2020, 3:08 p.m. UTC 
(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.

====

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

Comments

Nathan Sidwell April 27, 2020, 2 p.m. UTC | #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?
Iain Sandoe April 27, 2020, 6:41 p.m. UTC | #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;
+  }
+};
Nathan Sidwell April 27, 2020, 8:19 p.m. UTC | #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
diff mbox series

Patch

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;
+  }
+};