From patchwork Fri May 7 03:11:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jason Merrill X-Patchwork-Id: 1475311 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; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=esXu9wE8; dkim-atps=neutral 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 4FbwV35g5Hz9sXL for ; Fri, 7 May 2021 13:12:10 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C0359383581A; Fri, 7 May 2021 03:12:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C0359383581A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1620357127; bh=6Ua0F/ejdXGT/ux0m/I0Vj1GHven0gg5jrG91EAWnVI=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=esXu9wE8/ax9+g6noVEYzKVyOJ3xahdAcXW30Bv5R9y7/DkCQrAMlKs40/7ZanEbI gpFIUWiYyrKPgZUprEe4AZJKQ6035nWnUbMtP7CDACJntgsLhQtofG/C/ncS1eNWrf MW/9Y5hns21rcKNKf7wXZeXFIzxCF3ZrHzf9v+sI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id DC15F3835805 for ; Fri, 7 May 2021 03:12:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DC15F3835805 Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-4-dpQ2tt0yN8ufOvvUybdlvg-1; Thu, 06 May 2021 23:12:00 -0400 X-MC-Unique: dpQ2tt0yN8ufOvvUybdlvg-1 Received: by mail-qv1-f72.google.com with SMTP id r11-20020a0cb28b0000b02901c87a178503so5600110qve.22 for ; Thu, 06 May 2021 20:12:00 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=6Ua0F/ejdXGT/ux0m/I0Vj1GHven0gg5jrG91EAWnVI=; b=KBSE/IFnG20RRSGNT/KtxtzLUHqnyR5IKKGhaRfuU/60A4WrQduqQfNa7yLvSvFSSw iuy/MGn307NvaUYVc/ZcPGNG0swtzF/gPEpcDoRCajLqn8kVI2RAP0I5nqp8oKnmaS6x tGoddpXvW97PrYHUvkdGDJ9MKPpeCEAKkO78bky5rpAh5OmiBCY5fPBwBptw/M8EPa/E Kc/dfAqkskVUMmixwpjugtsC7khwC1087oVODTVN/wkEO8WLjsqQNq29t3j9eNWtXoe3 Gg76nS/Fo20cKXxOcElXyGWOIiHxCtoLJw2Am8Pk3Do6PryjfVwn1XnJ+n9VOWo3ArOO HufQ== X-Gm-Message-State: AOAM530G0jseHlXLP869eAPwO7G0vrANpDwE8Khqt6xGdjIL/tV0YJPr lY9F4LdH+Lyv0F4/MoKUpwJocQlZMpHS0x1xJ+83E64xiI9Dxkl/GmGcgG89syFpLTzUXX7ZXYX wyyTVg3WkxdmqOe0rjbhLayBQR/NKF7BL29ycKIMwH4O31qYI0yvQxvPtPiAFiuwj2Q== X-Received: by 2002:a05:622a:c8:: with SMTP id p8mr7380276qtw.145.1620357120025; Thu, 06 May 2021 20:12:00 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzXPCRHS3vEdmq8rbFiRhcN1n8LqdLTsxw7ca3DwxBOPC7u3UNPAIavybrbgk0jLwbCfxpP6w== X-Received: by 2002:a05:622a:c8:: with SMTP id p8mr7380248qtw.145.1620357119559; Thu, 06 May 2021 20:11:59 -0700 (PDT) Received: from barrymore.redhat.com (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id h10sm3819726qkh.47.2021.05.06.20.11.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 May 2021 20:11:58 -0700 (PDT) To: gcc-patches@gcc.gnu.org Subject: [PATCH RFC] c++: don't call 'rvalue' in coroutines code Date: Thu, 6 May 2021 23:11:56 -0400 Message-Id: <20210507031156.4023693-1-jason@redhat.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-15.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, 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: , X-Patchwork-Original-From: Jason Merrill via Gcc-patches From: Jason Merrill Reply-To: Jason Merrill Cc: Iain Sandoe Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" A change to check glvalue_p rather than specifically for TARGET_EXPR revealed issues with the coroutines code's use of the 'rvalue' function, which shouldn't be used on class glvalues, so I've removed those calls. In build_co_await I just dropped them, because I don't see anything in the co_await specification that indicates that we would want to move from an lvalue result of operator co_await. And simplified that code while I was touching it; cp_build_modify_expr (...INIT_EXPR...) will call the constructor. In morph_fn_to_coro I changed the handling of the rvalue reference coroutine frame field to use move, to treat the rval ref as an xvalue. I used forward_parm to pass the function parms to the constructor for the field. And I simplified the return handling so we get the desired rvalue semantics from the normal implicit move on return. Iain, does this all make sense to you? Incidentally, what is the standard citation for default-initializing the non-void return value of the function if get_return_object returns void? All I see is "The expression /promise/.get_return_object() is used to initialize the glvalue result or prvalue result object of a call to a coroutine", which suggests to me that that situation should be ill-formed. gcc/cp/ChangeLog: * coroutines.cc (build_co_await): Don't call 'rvalue'. (flatten_await_stmt): Simplify initialization. (morph_fn_to_coro): Change 'rvalue' to 'move'. Simplify. gcc/testsuite/ChangeLog: * g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C: Adjust diagnostic. --- gcc/cp/coroutines.cc | 117 +++++------------- .../coro-bad-gro-00-class-gro-scalar-return.C | 2 +- 2 files changed, 31 insertions(+), 88 deletions(-) base-commit: e82e87a851cdea9f4f43f342842025b068287d4e diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index dbd703a67cc..03543d5c112 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -950,18 +950,11 @@ build_co_await (location_t loc, tree a, suspend_point_kind suspend_kind) e_proxy = o; o = NULL_TREE; /* The var is already present. */ } - else if (type_build_ctor_call (o_type)) - { - e_proxy = get_awaitable_var (suspend_kind, o_type); - releasing_vec arg (make_tree_vector_single (rvalue (o))); - o = build_special_member_call (e_proxy, complete_ctor_identifier, - &arg, o_type, LOOKUP_NORMAL, - tf_warning_or_error); - } else { e_proxy = get_awaitable_var (suspend_kind, o_type); - o = build2 (INIT_EXPR, o_type, e_proxy, rvalue (o)); + o = cp_build_modify_expr (loc, e_proxy, INIT_EXPR, o, + tf_warning_or_error); } /* I suppose we could check that this is contextually convertible to bool. */ @@ -2989,15 +2982,8 @@ flatten_await_stmt (var_nest_node *n, hash_set *promoted, gcc_checking_assert (!already_present); tree inner = TREE_OPERAND (init, 1); gcc_checking_assert (TREE_CODE (inner) != COND_EXPR); - if (type_build_ctor_call (var_type)) - { - releasing_vec p_in (make_tree_vector_single (init)); - init = build_special_member_call (var, complete_ctor_identifier, - &p_in, var_type, LOOKUP_NORMAL, - tf_warning_or_error); - } - else - init = build2 (INIT_EXPR, var_type, var, init); + init = cp_build_modify_expr (input_location, var, INIT_EXPR, init, + tf_warning_or_error); /* Simplify for the case that we have an init containing the temp alone. */ if (t == n->init && n->var == NULL_TREE) @@ -4862,43 +4848,19 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) vec_safe_push (promise_args, this_ref); } else if (parm.rv_ref) - vec_safe_push (promise_args, rvalue(fld_idx)); + vec_safe_push (promise_args, move (fld_idx)); else vec_safe_push (promise_args, fld_idx); if (parm.rv_ref || parm.pt_ref) /* Initialise the frame reference field directly. */ - r = build_modify_expr (fn_start, TREE_OPERAND (fld_idx, 0), - parm.frame_type, INIT_EXPR, - DECL_SOURCE_LOCATION (arg), arg, - DECL_ARG_TYPE (arg)); - else if (type_build_ctor_call (parm.frame_type)) - { - vec *p_in; - if (CLASS_TYPE_P (parm.frame_type) - && classtype_has_non_deleted_move_ctor (parm.frame_type)) - p_in = make_tree_vector_single (move (arg)); - else if (lvalue_p (arg)) - p_in = make_tree_vector_single (rvalue (arg)); - else - p_in = make_tree_vector_single (arg); - /* Construct in place or move as relevant. */ - r = build_special_member_call (fld_idx, complete_ctor_identifier, - &p_in, parm.frame_type, - LOOKUP_NORMAL, - tf_warning_or_error); - release_tree_vector (p_in); - } + r = cp_build_modify_expr (fn_start, TREE_OPERAND (fld_idx, 0), + INIT_EXPR, arg, tf_warning_or_error); else { - if (!same_type_p (parm.frame_type, DECL_ARG_TYPE (arg))) - r = build1_loc (DECL_SOURCE_LOCATION (arg), CONVERT_EXPR, - parm.frame_type, arg); - else - r = arg; - r = build_modify_expr (fn_start, fld_idx, parm.frame_type, - INIT_EXPR, DECL_SOURCE_LOCATION (arg), r, - TREE_TYPE (r)); + r = forward_parm (arg); + r = cp_build_modify_expr (fn_start, fld_idx, INIT_EXPR, r, + tf_warning_or_error); } finish_expr_stmt (r); if (!parm.trivial_dtor) @@ -5044,16 +5006,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) DECL_IGNORED_P (gro) = true; add_decl_expr (gro); gro_bind_vars = gro; - if (type_build_ctor_call (gro_type)) - { - vec *arg = make_tree_vector_single (get_ro); - r = build_special_member_call (gro, complete_ctor_identifier, - &arg, gro_type, LOOKUP_NORMAL, - tf_warning_or_error); - release_tree_vector (arg); - } - else - r = build2_loc (fn_start, INIT_EXPR, gro_type, gro, get_ro); + r = cp_build_modify_expr (input_location, gro, INIT_EXPR, get_ro, + tf_warning_or_error); /* The constructed object might require a cleanup. */ if (TYPE_HAS_NONTRIVIAL_DESTRUCTOR (gro_type)) { @@ -5111,37 +5065,26 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) if (same_type_p (gro_type, fn_return_type)) r = gro_is_void_p ? NULL_TREE : DECL_RESULT (orig); + else if (!gro_is_void_p) + /* check_return_expr will automatically return gro as an rvalue via + treat_lvalue_as_rvalue_p. */ + r = gro; + else if (CLASS_TYPE_P (fn_return_type)) + { + /* For class type return objects, we can attempt to construct, + even if the gro is void. */ + r = build_special_member_call (NULL_TREE, + complete_ctor_identifier, NULL, + fn_return_type, LOOKUP_NORMAL, + tf_warning_or_error); + r = build_cplus_new (fn_return_type, r, tf_warning_or_error); + } else { - if (CLASS_TYPE_P (fn_return_type)) - { - /* For class type return objects, we can attempt to construct, - even if the gro is void. */ - vec *args = NULL; - vec **arglist = NULL; - if (!gro_is_void_p) - { - args = make_tree_vector_single (rvalue (gro)); - arglist = &args; - } - r = build_special_member_call (NULL_TREE, - 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); - if (args) - release_tree_vector (args); - } - else if (gro_is_void_p) - { - /* We can't initialize a non-class return value from void. */ - error_at (input_location, "cannot initialize a return object of type" - " %qT with an rvalue of type %", fn_return_type); - r = error_mark_node; - } - else - r = build1_loc (input_location, CONVERT_EXPR, - fn_return_type, rvalue (gro)); + /* We can't initialize a non-class return value from void. */ + error_at (input_location, "cannot initialize a return object of type" + " %qT with an rvalue of type %", fn_return_type); + r = error_mark_node; } finish_return_stmt (r); diff --git a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C index f9e8a5f398b..0512f03f4d0 100644 --- a/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C +++ b/gcc/testsuite/g++.dg/coroutines/coro-bad-gro-00-class-gro-scalar-return.C @@ -37,7 +37,7 @@ my_coro (std::coroutine_handle<>& h) { PRINT ("coro1: about to return"); co_return; -} // { dg-error {'struct Thing' used where a 'int' was expected} } +} // { dg-error {cannot convert 'Thing' to 'int' in return} } int main () {