[PATCH] coroutines: Pass lvalues to user-defined operator new [PR 100772].

[Jason Merrill]

On 11/5/21 11:49, Iain Sandoe wrote:
> The wording of the standard has been clarified to be explicit that
> the the parameters to any user-defined operator-new in the promise
> class should be lvalues.
> 
> tested on x86_64 darwin, linux,
> OK for master and backports?
> thanks
> Iain
> 
> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
> 
> 	PR c++/100772
> 
> gcc/cp/ChangeLog:
> 
> 	* coroutines.cc (morph_fn_to_coro): Convert function parms
> 	from reference before constructing any operator-new args
> 	list.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/coroutines/pr100772-a.C: New test.
> 	* g++.dg/coroutines/pr100772-b.C: New test.
> ---
>   gcc/cp/coroutines.cc                         |  6 +-
>   gcc/testsuite/g++.dg/coroutines/pr100772-a.C | 77 ++++++++++++++++
>   gcc/testsuite/g++.dg/coroutines/pr100772-b.C | 93 ++++++++++++++++++++
>   3 files changed, 174 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100772-a.C
>   create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100772-b.C
> 
> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
> index 6db4b70f028..ab211201255 100644
> --- a/gcc/cp/coroutines.cc
> +++ b/gcc/cp/coroutines.cc
> @@ -4602,8 +4602,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
>   	If the lookup finds an allocation function in the scope of the promise
>   	type, overload resolution is performed on a function call created by
>   	assembling an argument list.  The first argument is the amount of space
> -	requested, and has type std::size_t.  The succeeding arguments are
> -	those of the original function.  */
> +	requested, and has type std::size_t.  The lvalues p1...pn are the
> +	succeeding arguments..  */
>         vec<tree, va_gc> *args = make_tree_vector ();
>         vec_safe_push (args, resizeable); /* Space needed.  */
>   
> @@ -4623,6 +4623,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer)
>   					       tf_warning_or_error);
>   	      vec_safe_push (args, this_ref);
>   	    }
> +	  else if (parm_i->rv_ref || parm_i->pt_ref)
> +	    vec_safe_push (args, convert_from_reference (arg));

Hmm, does this need to be conditional?  convert_from_reference has no 
effect on an expression that doesn't have reference type, it's used 
unconditionally in lots of places.

OK either way.

>   	  else
>   	    vec_safe_push (args, arg);
>   	}
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr100772-a.C b/gcc/testsuite/g++.dg/coroutines/pr100772-a.C
> new file mode 100644
> index 00000000000..a325d384fc3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr100772-a.C
> @@ -0,0 +1,77 @@
> +//  { dg-additional-options "-fsyntax-only " }
> +#ifdef __clang__
> +#include <experimental/coroutine>
> +namespace std {
> +  using namespace std::experimental;
> +}
> +#else
> +#include <coroutine>
> +#endif
> +
> +struct Task
> +{
> +    struct promise_type
> +    {
> +		void return_void() const noexcept {}
> +
> +		void* operator new(std::size_t, auto &&...args) noexcept
> +		{
> +            static_assert(sizeof...(args) > 0);
> +            static_assert(sizeof...(args) == 2);
> +
> +			return nullptr;
> +		}
> +
> +		void operator delete(void *, std::size_t) noexcept
> +		{
> +		}
> +
> +        static Task get_return_object_on_allocation_failure() noexcept
> +        {
> +            return {};
> +        }
> +
> +        Task get_return_object() noexcept
> +        {
> +            return Task{ *this };
> +        }
> +
> +        std::suspend_always initial_suspend() noexcept
> +        {
> +            return {};
> +        }
> +
> +        std::suspend_always final_suspend() noexcept
> +        {
> +            return {};
> +        }
> +
> +        void unhandled_exception() noexcept {}
> +    };
> +
> +    using promise_handle = std::coroutine_handle<promise_type>;
> +
> +    Task() = default;
> +    Task(promise_type & promise) noexcept
> +        : m_handle{ promise_handle::from_promise(promise) }
> +    {}
> +
> +    ~Task()
> +    {
> +        if (m_handle.address()) { m_handle.destroy(); }
> +    }
> +
> +    promise_handle m_handle{};
> +};
> +
> +
> +Task Foo(auto && ... args) noexcept
> +{
> +    co_return;
> +}
> +
> +int main()
> +{
> +    int v;
> +    Foo(v, 2134);
> +}
> diff --git a/gcc/testsuite/g++.dg/coroutines/pr100772-b.C b/gcc/testsuite/g++.dg/coroutines/pr100772-b.C
> new file mode 100644
> index 00000000000..6cdf8d1e529
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/coroutines/pr100772-b.C
> @@ -0,0 +1,93 @@
> +#ifdef __clang__
> +#include <experimental/coroutine>
> +namespace std {
> +  using namespace std::experimental;
> +}
> +#else
> +#include <coroutine>
> +#endif
> +#include <cstdio>
> +#include <typeinfo>
> +#include <cxxabi.h>  // needed for abi::__cxa_demangle
> +#include <memory>
> +
> +std::shared_ptr<char> cppDemangle(const char *abiName)
> +{
> +  int status;
> +  char *ret = abi::__cxa_demangle(abiName, 0, 0, &status);
> +
> +  /* NOTE: must free() the returned char when done with it! */
> +  std::shared_ptr<char> retval;
> +  retval.reset( (char *)ret, [](char *mem) { if (mem) free((void*)mem); } );
> +  return retval;
> +}
> +
> +template <typename T>
> +struct Id{};
> +struct Task
> +{
> +  struct promise_type
> +  {
> +    void return_void() const noexcept {}
> +
> +    static void is_int (std::string x) {
> +      if (x != "Id<int>")
> +	abort() ;
> +    }
> +    template <typename ... Args>
> +    void* operator new(std::size_t len, Args ...args) noexcept
> +      {
> +	(is_int (cppDemangle(typeid(Id<Args>).name()).get()), ...);
> +	(std::puts (cppDemangle(typeid(Id<Args>).name()).get()), ...);
> +	return nullptr;
> +      }
> +
> +        static Task get_return_object_on_allocation_failure() noexcept
> +        {
> +            return {};
> +        }
> +
> +        Task get_return_object() noexcept
> +        {
> +            return Task{ *this };
> +        }
> +
> +        std::suspend_always initial_suspend() noexcept
> +        {
> +            return {};
> +        }
> +
> +        std::suspend_always final_suspend() noexcept
> +        {
> +            return {};
> +        }
> +
> +        void unhandled_exception() noexcept {}
> +    };
> +
> +    using promise_handle = std::coroutine_handle<promise_type>;
> +
> +    Task() = default;
> +    Task(promise_type & promise) noexcept
> +        : m_handle{ promise_handle::from_promise(promise) }
> +    {}
> +
> +    ~Task()
> +    {
> +        if (m_handle.address()) { m_handle.destroy(); }
> +    }
> +
> +    promise_handle m_handle{};
> +};
> +
> +
> +Task Foo(auto && ... args) noexcept
> +{
> +    co_return;
> +}
> +
> +int main()
> +{
> +    int v;
> +    Foo(v, 2134);
> +}
>