c++: enable NRVO from inner block [PR51571]

Our implementation of the named return value optimization has been limited
to variables declared in the outermost block of the function, to avoid
needing to handle the case where the variable needs to be destroyed due to
going out of scope.  PR92407 pointed out a case we were missing, where the
variable goes out of scope due to a goto and we were failing to destroy it.

It occurred to me that this problem is the flip side of PR33799, where we
need to be sure to destroy the return value if a cleanup throws on return;
here we want to avoid destroying the return value when exiting the
variable's scope on return.  We can use the same flag to indicate to both
cleanups that we're returning.

This implements the guaranteed copy elision specified by P2025 (which is not
yet part of the draft standard).

	PR c++/51571
	PR c++/92407

gcc/cp/ChangeLog:

	* decl.cc (finish_function): Simplify NRV handling.
	* except.cc (maybe_set_retval_sentinel): Also set if NRV.
	(maybe_splice_retval_cleanup): Don't add the cleanup region
	if we don't need it.
	* semantics.cc (nrv_data): Add simple field.
	(finalize_nrv): Set it.
	(finalize_nrv_r): Check it and retval sentinel.
	* cp-tree.h (finalize_nrv): Adjust declaration.
	* typeck.cc (check_return_expr): Remove named_labels check.

gcc/testsuite/ChangeLog:

	* g++.dg/opt/nrv23.C: New test.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 834fdd12ef8753337edac9d8205c657361095b33..87572e3574d2d58ea46598a69feef23a7a18795c 100644 (file)
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7714,7 +7714,7 @@ extern bool check_accessibility_of_qualified_id (tree, tree, tree, tsubst_flags_
 extern tree finish_qualified_id_expr           (tree, tree, bool, bool,
                                                 bool, bool, tsubst_flags_t);
 extern void simplify_aggr_init_expr            (tree *);
-extern void finalize_nrv                       (tree *, tree, tree);
+extern void finalize_nrv                       (tree, tree);
 extern tree omp_reduction_id                   (enum tree_code, tree, tree);
 extern tree cp_remove_omp_priv_cleanup_stmt    (tree *, int *, void *);
 extern bool cp_check_omp_declare_reduction     (tree);

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 3985c6d2d1f1cbdaa1d7f1a1715b419a8243abc0..c07a4a8d58d5bb4272eb91636eeb64296e544e8e 100644 (file)
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -18236,23 +18236,10 @@ finish_function (bool inline_p)
 
   /* Set up the named return value optimization, if we can.  Candidate
      variables are selected in check_return_expr.  */
-  if (current_function_return_value)
-    {
-      tree r = current_function_return_value;
-      tree outer;
-
-      if (r != error_mark_node
-         /* This is only worth doing for fns that return in memory--and
-            simpler, since we don't have to worry about promoted modes.  */
-         && aggregate_value_p (TREE_TYPE (TREE_TYPE (fndecl)), fndecl)
-         /* Only allow this for variables declared in the outer scope of
-            the function so we know that their lifetime always ends with a
-            return; see g++.dg/opt/nrv6.C.  We could be more flexible if
-            we were to do this optimization in tree-ssa.  */
-         && (outer = outer_curly_brace_block (fndecl))
-         && chain_member (r, BLOCK_VARS (outer)))
-       finalize_nrv (&DECL_SAVED_TREE (fndecl), r, DECL_RESULT (fndecl));
-
+  if (tree r = current_function_return_value)
+    {
+      if (r != error_mark_node)
+       finalize_nrv (fndecl, r);
       current_function_return_value = NULL_TREE;
     }
 

diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index 28106dadf1ebda461a658189ea355b3deb1272a8..6c0f0815424c1b0ae5bc145add7b107d05e355d1 100644 (file)
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -1280,7 +1280,9 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
 /* If the current function has a cleanup that might throw, and the return value
    has a non-trivial destructor, return a MODIFY_EXPR to set
    current_retval_sentinel so that we know that the return value needs to be
-   destroyed on throw.  Otherwise, returns NULL_TREE.  */
+   destroyed on throw.  Do the same if the current function might use the
+   named return value optimization, so we don't destroy it on return.
+   Otherwise, returns NULL_TREE.  */
 
 tree
 maybe_set_retval_sentinel ()
@@ -1290,7 +1292,9 @@ maybe_set_retval_sentinel ()
   tree retval = DECL_RESULT (current_function_decl);
   if (!TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (retval)))
     return NULL_TREE;
-  if (!cp_function_chain->throwing_cleanup)
+  if (!cp_function_chain->throwing_cleanup
+      && (current_function_return_value == error_mark_node
+         || current_function_return_value == NULL_TREE))
     return NULL_TREE;
 
   if (!current_retval_sentinel)
@@ -1338,6 +1342,10 @@ maybe_splice_retval_cleanup (tree compound_stmt, bool is_try)
          tsi_link_before (&iter, decl_expr, TSI_SAME_STMT);
        }
 
+      if (!cp_function_chain->throwing_cleanup)
+       /* We're only using the sentinel for an NRV.  */
+       return;
+
       /* Skip past other decls, they can't contain a return.  */
       while (TREE_CODE (tsi_stmt (iter)) == DECL_EXPR)
        tsi_next (&iter);

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index a13c16f34a3fc60c6abd735b84e8fd785c4dc441..1d397b6f25789f4cbde5fe3fd801cdcbb9c25d73 100644 (file)
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -4938,6 +4938,7 @@ public:
   tree var;
   tree result;
   hash_table<nofree_ptr_hash <tree_node> > visited;
+  bool simple;
 };
 
 /* Helper function for walk_tree, used by finalize_nrv below.  */
@@ -4952,6 +4953,9 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
      non-statements, except that we have to consider STMT_EXPRs.  */
   if (TYPE_P (*tp))
     *walk_subtrees = 0;
+  /* 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,
      but differs from using NULL_TREE in that it indicates that we care
      about the value of the RESULT_DECL.  But preserve anything appended
@@ -4965,11 +4969,20 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
                           && TREE_OPERAND (*p, 0) == dp->result);
       *p = dp->result;
     }
-  /* Change all cleanups for the NRV to only run when an exception is
-     thrown.  */
+  /* Change all cleanups for the NRV to only run when not returning.  */
   else if (TREE_CODE (*tp) == CLEANUP_STMT
           && CLEANUP_DECL (*tp) == dp->var)
-    CLEANUP_EH_ONLY (*tp) = 1;
+    {
+      if (dp->simple)
+       CLEANUP_EH_ONLY (*tp) = true;
+      else
+       {
+         tree cond = build3 (COND_EXPR, void_type_node,
+                             current_retval_sentinel,
+                             void_node, CLEANUP_EXPR (*tp));
+         CLEANUP_EXPR (*tp) = cond;
+       }
+    }
   /* Replace the DECL_EXPR for the NRV with an initialization of the
      RESULT_DECL, if needed.  */
   else if (TREE_CODE (*tp) == DECL_EXPR
@@ -5009,9 +5022,10 @@ finalize_nrv_r (tree* tp, int* walk_subtrees, void* data)
    RESULT_DECL for the function.  */
 
 void
-finalize_nrv (tree *tp, tree var, tree result)
+finalize_nrv (tree fndecl, tree var)
 {
   class nrv_data data;
+  tree result = DECL_RESULT (fndecl);
 
   /* Copy name from VAR to RESULT.  */
   DECL_NAME (result) = DECL_NAME (var);
@@ -5025,7 +5039,14 @@ finalize_nrv (tree *tp, tree var, tree result)
 
   data.var = var;
   data.result = result;
-  cp_walk_tree (tp, finalize_nrv_r, &data, 0);
+
+  /* This is simpler for variables declared in the outer scope of
+     the function so we know that their lifetime always ends with a
+     return; see g++.dg/opt/nrv6.C.  */
+  tree outer = outer_curly_brace_block (fndecl);
+  data.simple = chain_member (var, BLOCK_VARS (outer));
+
+  cp_walk_tree (&DECL_SAVED_TREE (fndecl), finalize_nrv_r, &data, 0);
 }
 \f
 /* Create CP_OMP_CLAUSE_INFO for clause C.  Returns true if it is invalid.  */

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 6618c6a20218a8110efcfb790931ee8e37f8b460..11fcc7fcd3b35df92906e0bd72811dac43ae9e9c 100644 (file)
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -11155,9 +11155,6 @@ check_return_expr (tree retval, bool *no_warning)
   if (fn_returns_value_p && flag_elide_constructors)
     {
       if (named_return_value_okay_p
-         /* The current NRV implementation breaks if a backward goto needs to
-            destroy the object (PR92407).  */
-         && !cp_function_chain->x_named_labels
           && (current_function_return_value == NULL_TREE
              || current_function_return_value == bare_retval))
        current_function_return_value = bare_retval;

diff --git a/gcc/testsuite/g++.dg/opt/nrv23.C b/gcc/testsuite/g++.dg/opt/nrv23.C
new file mode 100644 (file)
index 0000000..9e1253c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/nrv23.C
@@ -0,0 +1,23 @@
+// PR c++/51571
+// { dg-do link }
+
+int copies;
+
+struct A {
+  int i;
+  A(int i) : i(i) { }
+  A(A const &); // not defined
+};
+
+A h()
+{
+  {
+    A a(0);
+    return a;
+  }
+}
+
+int main()
+{
+  A c = h();
+}