From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A1BB03858C1F for ; Wed, 7 Jun 2023 22:06:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A1BB03858C1F Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686175581; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=t+bc8eAnNDuUKRcp2kh9AzDWNwXCra5MgHPHiu6i48I=; b=CwY6SEBw3ytS+MzlQ4541SYdueXGIxzki12fWDTwN54eaboejf2FBnnKqzReZ/rftw55Ir USxLLV2BqfOmR7kT/9nwJ/336+DyuiRcVGcNAGzGlsZR85i6XvZowH80wMOfM4krOkUOFw 237elknQdqaXgH+SIjni2eU7lWEWH1A= Received: from mail-qt1-f200.google.com (mail-qt1-f200.google.com [209.85.160.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-252-zNB1LOCoOW65c-DxMka3qQ-1; Wed, 07 Jun 2023 18:06:20 -0400 X-MC-Unique: zNB1LOCoOW65c-DxMka3qQ-1 Received: by mail-qt1-f200.google.com with SMTP id d75a77b69052e-3f6a96a6170so114108281cf.2 for ; Wed, 07 Jun 2023 15:06:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686175579; x=1688767579; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=t+bc8eAnNDuUKRcp2kh9AzDWNwXCra5MgHPHiu6i48I=; b=cNMfvMsH1TQv+1sTvVmUguxkKgGacRC+L3BkD2H2j8q4W7z+nicvhwMPZkA4PGO4fo rjt2iju33YUMwXXrG1FeJTjGGFgIyz7+o9RelUiSYaf78f8eQisChPRVh3HNlZEDEyXw T7OWeTAGdZ8LJRM1AWqVzQmkFhZI/bW8v0/f3nPxzMbXRSZFynHKwPcR4XXUTTiqZrHS IMdaqSZKoTQaQWqxAm3tnzWZiFg0BDlME5batuLQz4ub/gI66oOcsaCRvC6WireiWgaK oE51KXShPVn+/br0kOxGavwf34CK3XR2Xn9REzx2W/tCI1JnMRvNdYmTWUWwI9ZMPKHl tbWg== X-Gm-Message-State: AC+VfDzm8YldqYk2RBHxfsxXcbK1m4Q3ArA17lyMiFYONro+kZvFdhXn kMFClYYW5AFP3jTAnlBqvLkrTU0vSFwM7S2fzbsrQQBe/01CFyaNqk+lAbxXaWeS8AIjE6oh2Fd du9wjU6phtG7erc3TOGc/gbXNsGtVrnnevTIeq+qeKkrLH1TbgC7iNiHeN0rdzvESek3+YnmXXg == X-Received: by 2002:a05:622a:354:b0:3f6:aff0:6df0 with SMTP id r20-20020a05622a035400b003f6aff06df0mr5487306qtw.36.1686175579192; Wed, 07 Jun 2023 15:06:19 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4nVegtiT029dbn3qhW01FOLVTbT/OHKEydkeV/q1dnY1uVL4T96BAFzfWV72koHuTFMMDQww== X-Received: by 2002:a05:622a:354:b0:3f6:aff0:6df0 with SMTP id r20-20020a05622a035400b003f6aff06df0mr5487279qtw.36.1686175578719; Wed, 07 Jun 2023 15:06:18 -0700 (PDT) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id ci7-20020a05622a260700b003ef5dc13bbbsm216027qtb.85.2023.06.07.15.06.17 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 07 Jun 2023 15:06:17 -0700 (PDT) From: Jason Merrill To: gcc-patches@gcc.gnu.org Subject: [pushed] c++: allow NRV and non-NRV returns [PR58487] Date: Wed, 7 Jun 2023 18:06:15 -0400 Message-Id: <20230607220615.2981121-1-jason@redhat.com> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-12.8 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_NONE,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Tested x86_64-pc-linux-gnu, applying to trunk. -- 8< -- Now that we support NRV from an inner block, we can also support non-NRV returns from other blocks, since once the NRV is out of scope a later return expression can't possibly alias it. This fixes 58487 and half-fixes 53637: now one of the returns is elided, but not the other. Fixing the remaining xfails in these testcases will require a very different approach, probably involving a full tree/block walk from finalize_nrv, and check_return_expr only adding to a list of potential return variables. PR c++/58487 PR c++/53637 gcc/cp/ChangeLog: * cp-tree.h (INIT_EXPR_NRV_P): New. * semantics.cc (finalize_nrv_r): Check it. * name-lookup.h (decl_in_scope_p): Declare. * name-lookup.cc (decl_in_scope_p): New. * typeck.cc (check_return_expr): Allow non-NRV returns if the NRV is no longer in scope. gcc/testsuite/ChangeLog: * g++.dg/opt/nrv26.C: New test. * g++.dg/opt/nrv26a.C: New test. * g++.dg/opt/nrv27.C: New test. --- gcc/cp/cp-tree.h | 5 +++++ gcc/cp/name-lookup.h | 1 + gcc/cp/name-lookup.cc | 22 ++++++++++++++++++ gcc/cp/semantics.cc | 8 +++---- gcc/cp/typeck.cc | 37 ++++++++++++++++++++++++------- gcc/testsuite/g++.dg/opt/nrv26.C | 19 ++++++++++++++++ gcc/testsuite/g++.dg/opt/nrv26a.C | 18 +++++++++++++++ gcc/testsuite/g++.dg/opt/nrv27.C | 23 +++++++++++++++++++ 8 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/opt/nrv26.C create mode 100644 gcc/testsuite/g++.dg/opt/nrv26a.C create mode 100644 gcc/testsuite/g++.dg/opt/nrv27.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 87572e3574d..83982233111 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -444,6 +444,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; REINTERPRET_CAST_P (in NOP_EXPR) ALIGNOF_EXPR_STD_P (in ALIGNOF_EXPR) OVL_DEDUP_P (in OVERLOAD) + INIT_EXPR_NRV_P (in INIT_EXPR) ATOMIC_CONSTR_MAP_INSTANTIATED_P (in ATOMIC_CONSTR) contract_semantic (in ASSERTION_, PRECONDITION_, POSTCONDITION_STMT) 1: IDENTIFIER_KIND_BIT_1 (in IDENTIFIER_NODE) @@ -4078,6 +4079,10 @@ struct GTY(()) lang_decl { #define DELETE_EXPR_USE_VEC(NODE) \ TREE_LANG_FLAG_1 (DELETE_EXPR_CHECK (NODE)) +/* True iff this represents returning a potential named return value. */ +#define INIT_EXPR_NRV_P(NODE) \ + TREE_LANG_FLAG_0 (INIT_EXPR_CHECK (NODE)) + #define CALL_OR_AGGR_INIT_CHECK(NODE) \ TREE_CHECK2 ((NODE), CALL_EXPR, AGGR_INIT_EXPR) diff --git a/gcc/cp/name-lookup.h b/gcc/cp/name-lookup.h index b3e708561d8..613745ba501 100644 --- a/gcc/cp/name-lookup.h +++ b/gcc/cp/name-lookup.h @@ -449,6 +449,7 @@ extern void resort_type_member_vec (void *, void *, extern vec *set_class_bindings (tree, int extra = 0); extern void insert_late_enum_def_bindings (tree, tree); extern tree innermost_non_namespace_value (tree); +extern bool decl_in_scope_p (tree); extern cxx_binding *outer_binding (tree, cxx_binding *, bool); extern void cp_emit_debug_info_for_using (tree, tree); diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index eb5c333b5ea..b8ca7306a28 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -7451,6 +7451,28 @@ innermost_non_namespace_value (tree name) return binding ? binding->value : NULL_TREE; } +/* True iff current_binding_level is within the potential scope of local + variable DECL. */ + +bool +decl_in_scope_p (tree decl) +{ + gcc_checking_assert (DECL_FUNCTION_SCOPE_P (decl)); + + tree name = DECL_NAME (decl); + + for (cxx_binding *iter = NULL; + (iter = outer_binding (name, iter, /*class_p=*/false)); ) + { + if (!LOCAL_BINDING_P (iter)) + return false; + if (iter->value == decl) + return true; + } + + return false; +} + /* Look up NAME in the current binding level and its superiors in the namespace of variables, functions and typedefs. Return a ..._DECL node of some kind representing its definition if there is only one diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc index 1d397b6f257..a2e74a5d2c7 100644 --- a/gcc/cp/semantics.cc +++ b/gcc/cp/semantics.cc @@ -4956,7 +4956,7 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) /* If there's a label, we might need to destroy the NRV on goto (92407). */ else if (TREE_CODE (*tp) == LABEL_EXPR) dp->simple = false; - /* Change all returns to just refer to the RESULT_DECL; this is a nop, + /* Change NRV returns to just refer to the RESULT_DECL; this is a nop, but differs from using NULL_TREE in that it indicates that we care about the value of the RESULT_DECL. But preserve anything appended by check_return_expr. */ @@ -4965,9 +4965,9 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data) tree *p = &TREE_OPERAND (*tp, 0); while (TREE_CODE (*p) == COMPOUND_EXPR) p = &TREE_OPERAND (*p, 0); - gcc_checking_assert (TREE_CODE (*p) == INIT_EXPR - && TREE_OPERAND (*p, 0) == dp->result); - *p = dp->result; + if (TREE_CODE (*p) == INIT_EXPR + && INIT_EXPR_NRV_P (*p)) + *p = dp->result; } /* Change all cleanups for the NRV to only run when not returning. */ else if (TREE_CODE (*tp) == CLEANUP_STMT diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 6b5705e806d..11927cbdf83 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -11147,11 +11147,15 @@ check_return_expr (tree retval, bool *no_warning) So, if this is a value-returning function that always returns the same local variable, remember it. - It might be nice to be more flexible, and choose the first suitable - variable even if the function sometimes returns something else, but - then we run the risk of clobbering the variable we chose if the other - returned expression uses the chosen variable somehow. And people expect - this restriction, anyway. (jason 2000-11-19) + We choose the first suitable variable even if the function sometimes + returns something else, but only if the variable is out of scope at the + other return sites, or else we run the risk of clobbering the variable we + chose if the other returned expression uses the chosen variable somehow. + + We don't currently do this if the first return is a non-variable, as it + would be complicated to determine whether an NRV selected later was in + scope at the point of the earlier return. We also don't currently support + multiple variables with non-overlapping scopes (53637). See finish_function and finalize_nrv for the rest of this optimization. */ tree bare_retval = NULL_TREE; @@ -11162,12 +11166,26 @@ check_return_expr (tree retval, bool *no_warning) } bool named_return_value_okay_p = want_nrvo_p (bare_retval, functype); - if (fn_returns_value_p && flag_elide_constructors) + if (fn_returns_value_p && flag_elide_constructors + && current_function_return_value != bare_retval) { if (named_return_value_okay_p - && (current_function_return_value == NULL_TREE - || current_function_return_value == bare_retval)) + && current_function_return_value == NULL_TREE) current_function_return_value = bare_retval; + else if (current_function_return_value + && VAR_P (current_function_return_value) + && !decl_in_scope_p (current_function_return_value)) + { + /* The earlier NRV is out of scope at this point, so it's safe to + leave it alone; the current return can't refer to it. */; + if (named_return_value_okay_p + && !warning_suppressed_p (current_function_decl, OPT_Wnrvo)) + { + warning (OPT_Wnrvo, "not eliding copy on return from %qD", + bare_retval); + suppress_warning (current_function_decl, OPT_Wnrvo); + } + } else { if ((named_return_value_okay_p @@ -11281,6 +11299,9 @@ check_return_expr (tree retval, bool *no_warning) retval = cp_build_init_expr (result, retval); } + if (current_function_return_value == bare_retval) + INIT_EXPR_NRV_P (retval) = true; + if (tree set = maybe_set_retval_sentinel ()) retval = build2 (COMPOUND_EXPR, void_type_node, retval, set); diff --git a/gcc/testsuite/g++.dg/opt/nrv26.C b/gcc/testsuite/g++.dg/opt/nrv26.C new file mode 100644 index 00000000000..3d2b12c5f6c --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/nrv26.C @@ -0,0 +1,19 @@ +// PR c++/58487 +// { dg-additional-options -Wnrvo } +// { dg-do link } + +struct A { + A() {} + A(const A&); +}; + +A test() { + if (true) { + A a; + return a; + } else { + return A(); + } +} + +int main() { } diff --git a/gcc/testsuite/g++.dg/opt/nrv26a.C b/gcc/testsuite/g++.dg/opt/nrv26a.C new file mode 100644 index 00000000000..208014a186a --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/nrv26a.C @@ -0,0 +1,18 @@ +// PR c++/58487 +// { dg-additional-options -Wnrvo } + +struct A { + A() {} + A(const A&); +}; + +A test() { + if (true) { + return A(); + } else { + A a; + return a; // { dg-bogus "not eliding" "" { xfail *-*-* } } + } +} + +int main() { } diff --git a/gcc/testsuite/g++.dg/opt/nrv27.C b/gcc/testsuite/g++.dg/opt/nrv27.C new file mode 100644 index 00000000000..f71dd7c3eb2 --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/nrv27.C @@ -0,0 +1,23 @@ +// PR c++/53637 +// { dg-additional-options "-Wnrvo -fdump-tree-gimple" } + +class Foo { +public: + Foo() {} + Foo(const Foo& other); +}; + +Foo bar(int i) { + if (i > 1) { + Foo result; + return result; // We currently elide this copy, but not the second one. + // { dg-final { scan-tree-dump {result .value-expr} "gimple" } } + } else { + Foo result; + return result; // { dg-bogus "not eliding copy on return from" "" { xfail *-*-* } } + } +} + +int main(int argc, char* argv[]) { + Foo f = bar(argc); +} base-commit: 5faaabef3819434d13fcbf749bd07bfc98ca7c3c -- 2.31.1