Message ID | 12CF97D9-1D26-4B9F-A7D3-FC6D448BB1D7@sandoe.co.uk |
---|---|
State | New |
Headers | show |
Series | coroutines: Handle lambda closure pointers like 'this'. | expand |
On 5/22/20 8:27 AM, Iain Sandoe wrote: > Hi, > > We didn’t quite have time to push this through before the 10.1 > deadline, but (since it’s a correctness issue) it should be applied > to the branch too. > > tested on x86_64-darwin, linux > OK for master? > 10.2 after some bake time? > thanks > Iain > > ======= > > It was agreed amongst the implementors that the correct > interpretation of the standard is that lambda closure pointers > should be treated in the same manner as class object pointers. > > gcc/cp/ChangeLog: > > * coroutines.cc (instantiate_coro_traits): Pass a reference > to lambda closure objects to traits instantiation. > (morph_fn_to_coro): Likewise for promise parameter > preview and allocator lookup. > --- > gcc/cp/coroutines.cc | 19 +++++-------------- > 1 file changed, 5 insertions(+), 14 deletions(-) > > diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc > index b79e2c66b70..6e3f03bdb0d 100644 > --- a/gcc/cp/coroutines.cc > +++ b/gcc/cp/coroutines.cc > @@ -304,8 +304,8 @@ instantiate_coro_traits (tree fndecl, location_t kw) > > while (arg_node != NULL_TREE && !VOID_TYPE_P (TREE_VALUE (arg_node))) > { > - /* See PR94807, as to why we must exclude lambda here. */ > - if (is_this_parameter (arg) && !lambda_p) > + if (is_this_parameter (arg) > + || (lambda_p && DECL_NAME (arg) == closure_identifier)) DECL_NAME (arg) == closure_identifier is sufficient -- it's in the implementor's namespace. Just like how is_this_parameter works. (that I fixed trunk to not label a lambda's instantiation a this pointer doesn't matter here, so you don't have to account for it not being so on 10.2). > { > /* We pass a reference to *this to the param preview. */ > tree ct = TREE_TYPE (TREE_TYPE (arg)); > @@ -3793,13 +3793,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) > parm.frame_type = actual_type; > > parm.this_ptr = is_this_parameter (arg); > - /* See PR94807. When a lambda is in a template instantiation, the > - closure object is named 'this' instead of '__closure'. */ > if (lambda_p) > - { > - parm.lambda_cobj = DECL_NAME (arg) == closure_identifier; > - gcc_checking_assert (!parm.this_ptr); > - } > + parm.lambda_cobj = DECL_NAME (arg) == closure_identifier; but this bit will be different on 10.2. I think you want if (lambda_p) { parm.lambda_cobj = parm.this_ptr || DECL_NAME (arg) == closure_identifier; parm.this_pointer = false; } ? looks ok otherwise, for both.
diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index b79e2c66b70..6e3f03bdb0d 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -304,8 +304,8 @@ instantiate_coro_traits (tree fndecl, location_t kw) while (arg_node != NULL_TREE && !VOID_TYPE_P (TREE_VALUE (arg_node))) { - /* See PR94807, as to why we must exclude lambda here. */ - if (is_this_parameter (arg) && !lambda_p) + if (is_this_parameter (arg) + || (lambda_p && DECL_NAME (arg) == closure_identifier)) { /* We pass a reference to *this to the param preview. */ tree ct = TREE_TYPE (TREE_TYPE (arg)); @@ -3793,13 +3793,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) parm.frame_type = actual_type; parm.this_ptr = is_this_parameter (arg); - /* See PR94807. When a lambda is in a template instantiation, the - closure object is named 'this' instead of '__closure'. */ if (lambda_p) - { - parm.lambda_cobj = DECL_NAME (arg) == closure_identifier; - gcc_checking_assert (!parm.this_ptr); - } + parm.lambda_cobj = DECL_NAME (arg) == closure_identifier; else parm.lambda_cobj = false; @@ -3953,9 +3948,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) { 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) + if (parm_i->this_ptr || parm_i->lambda_cobj) { /* We pass a reference to *this to the allocator lookup. */ tree tt = TREE_TYPE (TREE_TYPE (arg)); @@ -4159,9 +4152,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* Add this to the promise CTOR arguments list, accounting for refs and special handling for method this ptr. */ - if (parm.lambda_cobj) - vec_safe_push (promise_args, arg); - else if (parm.this_ptr) + if (parm.this_ptr || parm.lambda_cobj) { /* We pass a reference to *this to the param preview. */ tree tt = TREE_TYPE (arg);