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

[Nathan Sidwell]

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  <i...@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  <i...@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