From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp002.apm-internet.net (smtp002.apm-internet.net [85.119.248.221]) by sourceware.org (Postfix) with ESMTPS id 4B5DB3858422 for ; Thu, 18 Nov 2021 23:43:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4B5DB3858422 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=sandoe.co.uk Received: (qmail 61353 invoked from network); 18 Nov 2021 23:43:00 -0000 X-APM-Out-ID: 16372789806135 X-APM-Authkey: 257869/1(257869/1) 4 Received: from unknown (HELO ?192.168.1.214?) (81.138.1.83) by smtp002.apm-internet.net with SMTP; 18 Nov 2021 23:43:00 -0000 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.21\)) Subject: Re: [PATCH] coroutines: Handle initial awaiters with non-void returns [PR 100127]. From: Iain Sandoe In-Reply-To: <6b457227-1ffb-cd4c-66fc-12639ade8612@redhat.com> Date: Thu, 18 Nov 2021 23:42:59 +0000 Cc: gcc-patches@gcc.gnu.org Content-Transfer-Encoding: quoted-printable Message-Id: References: <20211105154651.77786-1-iain@sandoe.co.uk> <6b457227-1ffb-cd4c-66fc-12639ade8612@redhat.com> To: Jason Merrill X-Mailer: Apple Mail (2.3445.104.21) X-Spam-Status: No, score=-15.0 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_COUK, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Nov 2021 23:43:04 -0000 > On 18 Nov 2021, at 22:13, Jason Merrill via Gcc-patches = wrote: >=20 > 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 >> 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 =3D TREE_OPERAND (initial_await, 0); >> tree vec =3D TREE_OPERAND (initial_await, 3); >> tree aw_r =3D TREE_VEC_ELT (vec, 2); >> + if (!VOID_TYPE_P (TREE_TYPE (aw_r))) >> + aw_r =3D build1 (CONVERT_EXPR, void_type_node, aw_r); >=20 > Is there a reason not to use convert_to_void? no, just me still learning APIs=E2=80=A6 I=E2=80=99ll do a revised and = check it. Iain >=20 >> tree update =3D build2 (MODIFY_EXPR, boolean_type_node, = i_a_r_c, >> boolean_true_node); >> aw_r =3D 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 >> +namespace std { >> + using namespace std::experimental; >> +} >> +#else >> +#include >> +#endif >> +#include >> + >> +struct future >> +{ >> + using value_type =3D int; >> + struct promise_type; >> + using handle_type =3D std::coroutine_handle; >> + >> + handle_type _coroutine; >> + >> + future(handle_type h) : _coroutine{h} {} >> + >> + ~future() noexcept{ >> + if (_coroutine) { >> + _coroutine.destroy(); >> + } >> + } >> + >> + value_type get() { >> + auto ptr =3D _coroutine.promise()._value; >> + return *ptr; >> + } >> + >> + struct promise_type { >> + std::optional _value =3D std::nullopt; >> + >> + future get_return_object() { >> + return future{handle_type::from_promise(*this)}; >> + } >> + void return_value(value_type val) { >> + _value =3D static_cast(val); >> + } >> + auto initial_suspend() noexcept { >> + class awaiter { >> + std::optional & value; >> + public: >> + explicit awaiter(std::optional & 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 =3D create_future(); }