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.

5 files changed, 41 insertions, 28 deletions

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 834fdd1..87572e3 100644
--- 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 3985c6d..c07a4a8 100644
--- 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 28106da..6c0f081 100644
--- 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 a13c16f..1d397b6 100644
--- 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);
 }
 
 /* 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 6618c6a..11fcc7f 100644
--- 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;