From patchwork Sat Apr 25 13:49:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 1276833 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 498XTz0r0Yz9sSG for ; Sat, 25 Apr 2020 23:49:58 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id BC85C3898520; Sat, 25 Apr 2020 13:49:56 +0000 (GMT) X-Original-To: GCC-patches@gcc.gnu.org Delivered-To: GCC-patches@gcc.gnu.org Received: from smtp1.wavenetuk.net (smtp.wavenetuk.net [195.26.36.10]) by sourceware.org (Postfix) with ESMTP id 48663386F442; Sat, 25 Apr 2020 13:49:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 48663386F442 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sandoe.co.uk Authentication-Results: sourceware.org; spf=none smtp.mailfrom=iain@sandoe.co.uk Received: from [192.168.1.212] (host81-138-1-83.in-addr.btopenworld.com [81.138.1.83]) by smtp1.wavenetuk.net (Postfix) with ESMTPA id F09101200492; Sat, 25 Apr 2020 14:49:51 +0100 (BST) From: Iain Sandoe Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [PATCH] coroutines: Fix handling of non-class coroutine returns [PR94759] Message-Id: Date: Sat, 25 Apr 2020 14:49:48 +0100 To: GCC-patches@gcc.gnu.org, libstdc++ X-Mailer: Apple Mail (2.3273) X-Spam-Status: No, score=-26.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_COUK, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , Cc: Jonathan Wakely , Nathan Sidwell Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi From the standard: The header defines the primary template coroutine_traits such that if ArgTypes is a parameter pack of types and if the qualified-id R::promise_type is valid and denotes a type, then coroutine_traits has the following publicly accessible member: using promise_type = typename R::promise_type; — however, this should not prevent more specialised cases and the code in the PR (the testcase in the following patch) should be accepted, but is currently rejected with: 'error: coroutine return type ‘void’ is not a class' This is because I applied the check for non-class-ness of the return value in the wrong place; it needs to be carried out in a SFINAE context. The following patch removes the restriction in the traits template instantiation and allows for the case that the ramp function could return void. Jonathan provided a draft of a change to the header to implement the required functionality, which has been incorporated. == tested on x86_64-darwin, OK for master if the testing passes regstrap on x86-64-linux? thanks Iain gcc/cp/ChangeLog: 2020-04-23 Iain Sandoe PR c++/94759 * coroutines.cc (coro_promise_type_found_p): Do not exclude non-classes here (this needs to be handled in the coroutine header). (morph_fn_to_coro): Allow for the case where the coroutine returns void. gcc/testsuite/ChangeLog: 2020-04-25 Iain Sandoe PR c++/94759 * g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C: Adjust for updated error messages. * g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C: Likewise. * g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C: Likewise. * g++.dg/coroutines/coro-missing-promise.C: Likewise. * g++.dg/coroutines/pr93458-5-bad-coro-type.C: Liekwise. * g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C: New test. libstdc++-v3/ChangeLog: 2020-04-25 Jonathan Wakely Iain Sandoe PR c++/94759 * include/std/coroutine: Implement handing for non- class coroutine return types. --- gcc/cp/coroutines.cc | 102 +++++++++--------- .../coroutines/coro-bad-alloc-00-bad-op-new.C | 2 +- .../coroutines/coro-bad-alloc-01-bad-op-del.C | 2 +- .../coro-bad-alloc-02-no-op-new-nt.C | 2 +- .../g++.dg/coroutines/coro-missing-promise.C | 2 - .../coroutines/pr93458-5-bad-coro-type.C | 4 +- .../torture/co-ret-17-void-ret-coro.C | 57 ++++++++++ libstdc++-v3/include/std/coroutine | 15 ++- 8 files changed, 122 insertions(+), 64 deletions(-) create mode 100644 gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index 4f254b7fd10..db6f16b3e21 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -441,18 +441,6 @@ coro_promise_type_found_p (tree fndecl, location_t loc) { /* Get the coroutine traits template class instance for the function signature we have - coroutine_traits */ - tree return_type = TREE_TYPE (TREE_TYPE (fndecl)); - if (!CLASS_TYPE_P (return_type)) - { - /* It makes more sense to show the function header for this, even - though we will have encountered it when processing a keyword. - Only emit the error once, not for every keyword we encounter. */ - if (!coro_info->coro_ret_type_error_emitted) - error_at (DECL_SOURCE_LOCATION (fndecl), "coroutine return type" - " %qT is not a class", return_type); - coro_info->coro_ret_type_error_emitted = true; - return false; - } tree templ_class = instantiate_coro_traits (fndecl, loc); @@ -3500,9 +3488,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (!coro_function_valid_p (orig)) return false; - /* The ramp function does return a value. */ - current_function_returns_value = 1; - /* We can't validly get here with an empty statement list, since there's no way for the FE to decide it's a coroutine in the absence of any code. */ tree fnbody = pop_stmt_list (DECL_SAVED_TREE (orig)); @@ -3575,7 +3560,6 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) 1. Types we already know. */ tree fn_return_type = TREE_TYPE (TREE_TYPE (orig)); - gcc_assert (!VOID_TYPE_P (fn_return_type)); tree handle_type = get_coroutine_handle_type (orig); tree promise_type = get_coroutine_promise_type (orig); @@ -3927,33 +3911,27 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) control to the caller of the coroutine and the return value is obtained by a call to T::get_return_object_on_allocation_failure(), where T is the promise type. */ - tree cfra_label - = create_named_label_with_ctx (fn_start, "coro.frame.active", - current_scope ()); - tree early_ret_list = NULL; - /* init the retval using the user's func. */ - r = build2 (INIT_EXPR, TREE_TYPE (DECL_RESULT (orig)), DECL_RESULT (orig), - grooaf); - r = coro_build_cvt_void_expr_stmt (r, fn_start); - append_to_statement_list (r, &early_ret_list); - /* We know it's the correct type. */ - r = DECL_RESULT (orig); - r = build_stmt (fn_start, RETURN_EXPR, r); - TREE_NO_WARNING (r) |= 1; - r = maybe_cleanup_point_expr_void (r); - append_to_statement_list (r, &early_ret_list); - - tree goto_st = NULL; - r = build1 (GOTO_EXPR, void_type_node, cfra_label); - append_to_statement_list (r, &goto_st); - - tree ckk = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node); - tree ckz = build2 (EQ_EXPR, boolean_type_node, coro_fp, ckk); - r = build3 (COND_EXPR, void_type_node, ckz, early_ret_list, empty_list); - add_stmt (r); - cfra_label = build_stmt (fn_start, LABEL_EXPR, cfra_label); - add_stmt (cfra_label); + gcc_checking_assert (same_type_p (fn_return_type, TREE_TYPE (grooaf))); + tree if_stmt = begin_if_stmt (); + tree cond = build1 (CONVERT_EXPR, coro_frame_ptr, integer_zero_node); + cond = build2 (EQ_EXPR, boolean_type_node, coro_fp, cond); + finish_if_stmt_cond (cond, if_stmt); + if (VOID_TYPE_P (fn_return_type)) + { + /* Execute the get-return-object-on-alloc-fail call... */ + finish_expr_stmt (grooaf); + /* ... but discard the result, since we return void. */ + finish_return_stmt (NULL_TREE); + } + else + { + /* Get the fallback return object. */ + r = build_cplus_new (fn_return_type, grooaf, tf_warning_or_error); + finish_return_stmt (r); + } + finish_then_clause (if_stmt); + finish_if_stmt (if_stmt); } /* deref the frame pointer, to use in member access code. */ @@ -4151,17 +4129,25 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) } tree gro_context_body = push_stmt_list (); - tree gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), - TREE_TYPE (get_ro)); - DECL_CONTEXT (gro) = current_scope (); - add_decl_expr (gro); - tree gro_bind_vars = gro; + bool gro_is_void_p = VOID_TYPE_P (TREE_TYPE (get_ro)); + tree gro, gro_bind_vars = NULL_TREE; /* We have to sequence the call to get_return_object before initial suspend. */ - r = build2_loc (fn_start, INIT_EXPR, TREE_TYPE (gro), gro, get_ro); - r = coro_build_cvt_void_expr_stmt (r, fn_start); - add_stmt (r); + if (gro_is_void_p) + finish_expr_stmt (get_ro); + else + { + gro = build_lang_decl (VAR_DECL, get_identifier ("coro.gro"), + TREE_TYPE (get_ro)); + DECL_CONTEXT (gro) = current_scope (); + add_decl_expr (gro); + gro_bind_vars = gro; + + r = build2_loc (fn_start, INIT_EXPR, TREE_TYPE (gro), gro, get_ro); + r = coro_build_cvt_void_expr_stmt (r, fn_start); + add_stmt (r); + } /* Initialize the resume_idx_name to 0, meaning "not started". */ tree resume_idx_m @@ -4197,16 +4183,24 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* The ramp is done, we just need the return value. */ if (!same_type_p (TREE_TYPE (get_ro), fn_return_type)) { - /* construct the return value with a single GRO param. */ - vec *args = make_tree_vector_single (gro); + /* construct the return value with a single GRO param, if it's not + void. */ + vec *args = NULL; + vec **arglist = NULL; + if (!gro_is_void_p) + { + args = make_tree_vector_single (gro); + arglist = &args; + } r = build_special_member_call (NULL_TREE, - complete_ctor_identifier, &args, + complete_ctor_identifier, arglist, fn_return_type, LOOKUP_NORMAL, tf_warning_or_error); r = build_cplus_new (fn_return_type, r, tf_warning_or_error); } - else + else if (!gro_is_void_p) r = rvalue (gro); /* The GRO is the return value. */ + else r = NULL_TREE; finish_return_stmt (r); diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C index 06543a98824..0bcaff6d84a 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-00-bad-op-new.C @@ -6,7 +6,7 @@ #include "coro1-allocators.h" struct coro1 -f () /* { dg-error {'operator new' is provided by 'std::__n4861::coroutine_traits::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */ +f () /* { dg-error {'operator new' is provided by 'std::__n4861::__coroutine_traits__impl::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */ { co_return; } diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C index 9a4ec34cdf3..385f99c3dfd 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-01-bad-op-del.C @@ -6,7 +6,7 @@ #include "coro1-allocators.h" struct coro1 -f () /* { dg-error {'operator delete' is provided by 'std::__n4861::coroutine_traits::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */ +f () /* { dg-error {'operator delete' is provided by 'std::__n4861::__coroutine_traits__impl::promise_type' \{aka 'coro1::promise_type'\} but is not usable with the function signature 'coro1 f\(\)'} } */ { co_return; } diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C index c0c0792d31d..faf2c55825e 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-alloc-02-no-op-new-nt.C @@ -9,7 +9,7 @@ #include "coro1-allocators.h" struct coro1 -f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::coroutine_traits::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */ +f () /* { dg-error {'coro1::promise_type::get_return_object_on_allocation_failure\(\)\(\)' is provided by 'std::__n4861::__coroutine_traits__impl::promise_type' \{aka 'coro1::promise_type'\} but 'operator new' is not marked 'throw\(\)' or 'noexcept'} } */ { co_return; } diff --git a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C index 3fc21a4e2ca..e75f2002db0 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-missing-promise.C @@ -4,8 +4,6 @@ // Diagnose completely missing promise. -// { dg-error {no type named 'promise_type' in 'struct NoPromiseHere'} "" { target *-*-* } 0 } - struct NoPromiseHere { coro::coroutine_handle<> handle; NoPromiseHere () : handle (nullptr) {} diff --git a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C index 8bb58cc0a78..765c04892ef 100644 --- a/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C +++ b/gcc/testsuite/g++.dg/coroutines/pr93458-5-bad-coro-type.C @@ -5,8 +5,8 @@ #include "coro.h" int -bad_coroutine (void) // { dg-error {coroutine return type 'int' is not a class} } +bad_coroutine (void) { - co_yield 5; + co_yield 5; // { dg-error {unable to find the promise type for this coroutine} } co_return; } diff --git a/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C new file mode 100644 index 00000000000..3168ea272c1 --- /dev/null +++ b/gcc/testsuite/g++.dg/coroutines/torture/co-ret-17-void-ret-coro.C @@ -0,0 +1,57 @@ +// { dg-do run } + +// Test the ability to specialize the coroutine traits to include +// non-class type coroutine ramp return values. + +#include "../coro.h" + +template +struct std::coroutine_traits { + struct promise_type { + promise_type (HandleRef h, T ...args) + { h = std::coroutine_handle::from_promise (*this); + PRINT ("Created Promise"); + } + + void get_return_object() {} + + auto initial_suspend() { + return std::suspend_always{}; + } + auto final_suspend() { return std::suspend_never{}; } + + void return_void() {} + void unhandled_exception() {} + }; +}; + +void +my_coro (std::coroutine_handle<>& h) +{ + PRINT ("coro1: about to return"); + co_return; +} + +int main () +{ + std::coroutine_handle<> h; + my_coro (h); + + if (h.done()) + { + PRINT ("main: apparently was already done..."); + abort (); + } + + // initial suspend. + h.resume (); + + if (!h.done()) + { + PRINT ("main: apparently wasn't done..."); + abort (); + } + + PRINT ("main: returning"); + return 0; +} diff --git a/libstdc++-v3/include/std/coroutine b/libstdc++-v3/include/std/coroutine index 4fa1355c0ca..89bcae8b09a 100644 --- a/libstdc++-v3/include/std/coroutine +++ b/libstdc++-v3/include/std/coroutine @@ -63,12 +63,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // 17.12.2 coroutine traits /// [coroutine.traits] /// [coroutine.traits.primary] - template - struct coroutine_traits + /// If _Result::promise_type is valid and denotes a type then the traits + /// have a single publicly accessible member, otherwise they are empty. + template + struct __coroutine_traits__impl {}; + + template + struct __coroutine_traits__impl<_Result, + __void_t> { - using promise_type = typename _Result::promise_type; + using promise_type = typename _Result::promise_type; }; + template + struct coroutine_traits : __coroutine_traits__impl<_Result> {}; + // 17.12.3 Class template coroutine_handle /// [coroutine.handle] template