[PATCH] coroutines: Pass class ref to traits lookup and promise allocator [PR94760]
Iain Sandoe
iain@sandoe.co.uk
Sat Apr 25 15:08:39 GMT 2020
(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
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;
+ }
+};
--
2.24.1
More information about the Gcc-patches
mailing list