From: Iain Sandoe <iain@sandoe.co.uk>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] coroutines: Handle initial awaiters with non-void returns [PR 100127].
Date: Thu, 18 Nov 2021 23:42:59 +0000 [thread overview]
Message-ID: <DFD981B9-F3E7-4BF7-B195-29DF07862B27@sandoe.co.uk> (raw)
In-Reply-To: <6b457227-1ffb-cd4c-66fc-12639ade8612@redhat.com>
> On 18 Nov 2021, at 22:13, Jason Merrill via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>
> On 11/5/21 11:46, Iain Sandoe wrote:
>> The way in which a C++20 coroutine is specified discards any value
>> that might be returned from the initial or final await expressions.
>> This PR ICE was caused by an initial await expression with an
>> await_resume () returning a reference, the function rewrite code
>> was not set up to expect this.
>> Fixed by looking through any indirection present and by explicitly
>> discarding the value, if any, returned by await_resume().
>> It does not seem useful to make a diagnostic for this, since
>> the user could define a generic awaiter that usefully returns
>> values when used in a different position from the initial (or
>> final) await expressions.
>> tested on x86_64 darwin, linux,
>> OK for master and backports?
>> thanks
>> Iain
>> Signed-off-by: Iain Sandoe <iain@sandoe.co.uk>
>> PR c++/100127
>> gcc/cp/ChangeLog:
>> * coroutines.cc (coro_rewrite_function_body): Handle initial
>> await expressions that try to produce a reference value.
>> gcc/testsuite/ChangeLog:
>> * g++.dg/coroutines/pr100127.C: New test.
>> ---
>> gcc/cp/coroutines.cc | 9 ++-
>> gcc/testsuite/g++.dg/coroutines/pr100127.C | 65 ++++++++++++++++++++++
>> 2 files changed, 73 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/g++.dg/coroutines/pr100127.C
>> diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc
>> index 9017902e6fb..6db4b70f028 100644
>> --- a/gcc/cp/coroutines.cc
>> +++ b/gcc/cp/coroutines.cc
>> @@ -4211,9 +4211,16 @@ coro_rewrite_function_body (location_t fn_start, tree fnbody, tree orig,
>> {
>> /* Build a compound expression that sets the
>> initial-await-resume-called variable true and then calls the
>> - initial suspend expression await resume. */
>> + initial suspend expression await resume.
>> + In the case that the user decides to make the initial await
>> + await_resume() return a value, we need to discard it and, it is
>> + a reference type, look past the indirection. */
>> + if (INDIRECT_REF_P (initial_await))
>> + initial_await = TREE_OPERAND (initial_await, 0);
>> tree vec = TREE_OPERAND (initial_await, 3);
>> tree aw_r = TREE_VEC_ELT (vec, 2);
>> + if (!VOID_TYPE_P (TREE_TYPE (aw_r)))
>> + aw_r = build1 (CONVERT_EXPR, void_type_node, aw_r);
>
> Is there a reason not to use convert_to_void?
no, just me still learning APIs… I’ll do a revised and check it.
Iain
>
>> tree update = build2 (MODIFY_EXPR, boolean_type_node, i_a_r_c,
>> boolean_true_node);
>> aw_r = cp_build_compound_expr (update, aw_r, tf_warning_or_error);
>> diff --git a/gcc/testsuite/g++.dg/coroutines/pr100127.C b/gcc/testsuite/g++.dg/coroutines/pr100127.C
>> new file mode 100644
>> index 00000000000..374cd710077
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/coroutines/pr100127.C
>> @@ -0,0 +1,65 @@
>> +#ifdef __clang__
>> +#include <experimental/coroutine>
>> +namespace std {
>> + using namespace std::experimental;
>> +}
>> +#else
>> +#include <coroutine>
>> +#endif
>> +#include <optional>
>> +
>> +struct future
>> +{
>> + using value_type = int;
>> + struct promise_type;
>> + using handle_type = std::coroutine_handle<promise_type>;
>> +
>> + handle_type _coroutine;
>> +
>> + future(handle_type h) : _coroutine{h} {}
>> +
>> + ~future() noexcept{
>> + if (_coroutine) {
>> + _coroutine.destroy();
>> + }
>> + }
>> +
>> + value_type get() {
>> + auto ptr = _coroutine.promise()._value;
>> + return *ptr;
>> + }
>> +
>> + struct promise_type {
>> + std::optional<value_type> _value = std::nullopt;
>> +
>> + future get_return_object() {
>> + return future{handle_type::from_promise(*this)};
>> + }
>> + void return_value(value_type val) {
>> + _value = static_cast<value_type &&>(val);
>> + }
>> + auto initial_suspend() noexcept {
>> + class awaiter {
>> + std::optional<value_type> & value;
>> + public:
>> + explicit awaiter(std::optional<value_type> & val) noexcept : value{val} {}
>> + bool await_ready() noexcept { return value.has_value(); }
>> + void await_suspend(handle_type) noexcept { }
>> + value_type & await_resume() noexcept { return *value; }
>> + };
>> +
>> + return awaiter{_value};
>> + }
>> + std::suspend_always final_suspend() noexcept {
>> + return {};
>> + }
>> + //void return_void() {}
>> + void unhandled_exception() {}
>> + };
>> +};
>> +
>> +future create_future()
>> +{ co_return 2021; }
>> +
>> +int main()
>> +{ auto f = create_future(); }
next prev parent reply other threads:[~2021-11-18 23:43 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-05 15:46 Iain Sandoe
2021-11-18 22:13 ` Jason Merrill
2021-11-18 23:42 ` Iain Sandoe [this message]
2021-11-19 17:40 ` Iain Sandoe
2021-11-23 19:48 ` Jason Merrill
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DFD981B9-F3E7-4BF7-B195-29DF07862B27@sandoe.co.uk \
--to=iain@sandoe.co.uk \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).