Message ID | 20211105154909.91162-1-iain@sandoe.co.uk |
---|---|
State | New |
Headers | show |
Series | coroutines: Pass lvalues to user-defined operator new [PR 100772]. | expand |
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); > +} >
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)); 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); +}
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