Hi Patrick
thanks for looking at this.
> On 22 Jul 2024, at 14:43, Patrick Palka <ppa...@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk
> and perhaps backports?
>
> -- >8 --
>
> When passing *this to the promise type ctor (or operator new) (as
> per [dcl.fct.def.coroutine]/4), we add an explicit cast to lvalue
> reference, but that's unnecessary since *this is already an lvalue.
> And it'd mean we'd have to call convert_from_reference afterwards to
> lower the reference-yielding expression into an implicit dereference,
> otherwise overload resolution gets confused when computing argument
> conversions. So this patch removes the unnecessary reference cast when
> passing *this to the promise ctor, and removes both the cast and
> implicit deref when passing *this to operator new, for consistency.
This LGTM (although I cannot approve it) for what the current WD says.
(this section changed wording a few times and some editions were somewhat
unclear about requiements).
I tried it on the cppcoro testsuite without regression - although cppcoro is
quite
small; not had a chance to see if we get any progressions on folly yet.
thanks
Iain
>
> PR c++/104981
> PR c++/115550
>
> gcc/cp/ChangeLog:
>
> * coroutines.cc (morph_fn_to_coro): Remove unnecessary calls
> to convert_to_reference and convert_from_reference for *this.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/coroutines/pr104981-preview-this.C: New test.
> * g++.dg/coroutines/pr115550-preview-this.C: New test.
> ---
> gcc/cp/coroutines.cc | 10 +---
> .../g++.dg/coroutines/pr104981-preview-this.C | 34 ++++++++++++++
> .../g++.dg/coroutines/pr115550-preview-this.C | 47 +++++++++++++++++++
> 3 files changed, 82 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr104981-preview-this.C
> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr115550-preview-this.C
>
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index f350fc33e9b..b8a53182c38 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4622,11 +4622,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
> *destroyer)
> /* 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, convert_from_reference (this_ref));
> + vec_safe_push (args, this_ref);
> }
> else
> vec_safe_push (args, convert_from_reference (arg));
> @@ -4849,10 +4845,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree
> *destroyer)
> gcc_checking_assert (POINTER_TYPE_P (tt));
> tree ct = TREE_TYPE (tt);
> tree this_ref = build1 (INDIRECT_REF, ct, arg);
> - tree rt = cp_build_reference_type (ct, false);
> - this_ref = convert_to_reference (rt, this_ref, CONV_STATIC,
> - LOOKUP_NORMAL, NULL_TREE,
> - tf_warning_or_error);
> vec_safe_push (promise_args, this_ref);
> }
> else if (parm.rv_ref)
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr104981-preview-this.C
> b/gcc/testsuite/g++.dg/coroutines/pr104981-preview-this.C
> new file mode 100644
> index 00000000000..81eb963db4a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr104981-preview-this.C
> @@ -0,0 +1,34 @@
> +// PR c++/104981 - ICE from convert_to_base when passing *this to promise
> ctor
> +
> +#include <coroutine>
> +
> +class Base {};
> +
> +struct PromiseType;
> +
> +struct Result {
> + using promise_type = PromiseType;
> +};
> +
> +struct PromiseType {
> + PromiseType(const Base& parser, auto&&...) {}
> +
> + Result get_return_object() { return {}; }
> +
> + static std::suspend_never initial_suspend() { return {}; }
> + static std::suspend_always final_suspend() noexcept { return {}; }
> + [[noreturn]] static void unhandled_exception() { throw; }
> +
> + void return_value(int) {}
> +};
> +
> +struct Derived : Base {
> + Result f() {
> + co_return 42;
> + }
> +};
> +
> +int main() {
> + Derived d;
> + d.f();
> +}
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr115550-preview-this.C
> b/gcc/testsuite/g++.dg/coroutines/pr115550-preview-this.C
> new file mode 100644
> index 00000000000..f62c07096b6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr115550-preview-this.C
> @@ -0,0 +1,47 @@
> +// PR c++/115550 - wrong deduction when passing *this to promise ctor
> template
> +
> +#include <coroutine>
> +
> +template <typename T> struct remove_reference { using type = T; };
> +template <typename T> struct remove_reference<T&> { using type = T; };
> +template <typename T> struct remove_reference<T&&> { using type = T; };
> +template <typename T> using remove_reference_t = remove_reference<T>::type;
> +
> +template <typename, typename>
> +struct is_same { static inline constexpr bool value = false; };
> +template <typename T>
> +struct is_same<T, T> { static inline constexpr bool value = true; };
> +
> +template <typename T, typename U>
> +concept same_as = is_same<T, U>::value;
> +
> +struct coroutine
> +{
> + struct promise_type
> + {
> + template <typename Arg>
> + explicit promise_type(Arg&&)
> + {
> + static_assert(same_as<
> + remove_reference_t<remove_reference_t<Arg>>,
> + remove_reference_t<Arg>
> + >);
> + }
> +
> + coroutine get_return_object() { return {}; }
> +
> + std::suspend_never initial_suspend() noexcept { return {}; }
> + std::suspend_never final_suspend() noexcept { return {}; }
> +
> + void return_void() {}
> + void unhandled_exception() {throw;}
> + };
> +};
> +
> +struct x
> +{
> + coroutine f()
> + {
> + co_return;
> + }
> +};
> --
> 2.46.0.rc0.106.g1c4a234a1c
>
