On 4/27/20 2:41 PM, Iain Sandoe wrote:
Nathan Sidwell <nat...@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  <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-27  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                          | 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

Reply via email to