From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2122) id C6FAA3858C1F; Wed, 7 Jun 2023 22:08:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C6FAA3858C1F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1686175724; bh=/TvMrigvim4qertCyfLQbycS/T7nwz/GFgoLmwQkhdQ=; h=From:To:Subject:Date:From; b=Ut9P3NrbHuYMBdQ1C91g5lz7Fn/V+ZFWLUN8lOxcdfmq6NtKRAgpOgavV71TKffBU TU0dAddP1EpRlxSVdAWueZAmjFbrWF/4yffmmyV+zLJAOasBzuH5b32o/fNO3jMHCY TN5SvBBIhT7IHQs2sxnL4HvhRi3ZsyZl4nu5Y3vg= MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jason Merrill To: gcc-cvs@gcc.gnu.org Subject: [gcc r14-1624] c++: allow NRV and non-NRV returns [PR58487] X-Act-Checkin: gcc X-Git-Author: Jason Merrill X-Git-Refname: refs/heads/trunk X-Git-Oldrev: 941209f9da23b4e0f338ed970012fcfa7b20e528 X-Git-Newrev: 28db36e2cfca1b7106adc8d371600fa3a325c4e2 Message-Id: <20230607220844.C6FAA3858C1F@sourceware.org> Date: Wed, 7 Jun 2023 22:08:44 +0000 (GMT) List-Id: https://gcc.gnu.org/g:28db36e2cfca1b7106adc8d371600fa3a325c4e2 commit r14-1624-g28db36e2cfca1b7106adc8d371600fa3a325c4e2 Author: Jason Merrill Date: Wed Jun 7 05:15:02 2023 -0400 c++: allow NRV and non-NRV returns [PR58487] 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. Diff: --- 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(-) 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); +}