diff mbox

Fix PR c++/18969 (invalid return statement diagnosed too late)

Message ID 1437878959-10982-1-git-send-email-patrick@parcs.ath.cx
State New
Headers show

Commit Message

Patrick Palka July 26, 2015, 2:49 a.m. UTC 
This patch makes check_return_expr less paranoid about checking the
return value of a return-statement when processing a template
declaration.

At the same time this patch removes some vestigial code (the middle two
hunks) that were related to the long-removed "named return value" gcc
extension.

In the PR, Manuel expressed concern about this change repeating the same
error multiple times, once at declaration time and then at instantiation
time.  But IMHO the benefit of emitting an early error at declaration
time far outweighs the inconvenience of that error possibly being
printed more than once.  Also it is already the case that the same such
error can be printed more than once if we instantiate a template more
than once (using different template arguments each time).

This patch helped to catch a potential error in libstdc++:

https://gcc.gnu.org/ml/libstdc++/2015-07/msg00053.html

OK to commit after bootstrap + regtest?

gcc/cp/ChangeLog:

	PR c++/18969
	* typeck.c (check_return_expr): Also do the basic return-value
	validity checking if processing_template_decl and yet types are
	not dependent.  Remove obsolete code.

gcc/testsuite/ChangeLog:

	PR c++/18969
	* g++.dg/template/pr18969.C: New test.
	* g++.dg/template/pr18969-2.C: New test.
---
 gcc/cp/typeck.c                           | 22 ++++++++++++++++------
 gcc/testsuite/g++.dg/template/pr18969-2.C | 11 +++++++++++
 gcc/testsuite/g++.dg/template/pr18969.C   | 14 ++++++++++++++
 3 files changed, 41 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/pr18969-2.C
 create mode 100644 gcc/testsuite/g++.dg/template/pr18969.C

Comments

Jason Merrill July 26, 2015, 3:46 a.m. UTC | #1 
OK.

Jason
diff mbox

Patch

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index b88a3fd..36a3ee7 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -8515,12 +8515,19 @@  check_return_expr (tree retval, bool *no_warning)
       return NULL_TREE;
     }
 
+  const tree saved_retval = retval;
+
   if (processing_template_decl)
     {
       current_function_returns_value = 1;
+
       if (check_for_bare_parameter_packs (retval))
-        retval = error_mark_node;
-      return retval;
+	return error_mark_node;
+
+      if (WILDCARD_TYPE_P (TREE_TYPE (DECL_RESULT (current_function_decl)))
+	  || (retval != NULL_TREE
+	      && type_dependent_expression_p (retval)))
+        return retval;
     }
 
   functype = TREE_TYPE (TREE_TYPE (current_function_decl));
@@ -8564,14 +8571,10 @@  check_return_expr (tree retval, bool *no_warning)
       functype = type;
     }
 
-  /* When no explicit return-value is given in a function with a named
-     return value, the named return value is used.  */
   result = DECL_RESULT (current_function_decl);
   valtype = TREE_TYPE (result);
   gcc_assert (valtype != NULL_TREE);
   fn_returns_value_p = !VOID_TYPE_P (valtype);
-  if (!retval && DECL_NAME (result) && fn_returns_value_p)
-    retval = result;
 
   /* Check for a return statement with no return value in a function
      that's supposed to return a value.  */
@@ -8656,6 +8659,13 @@  check_return_expr (tree retval, bool *no_warning)
 	warning (OPT_Weffc__, "%<operator=%> should return a reference to %<*this%>");
     }
 
+  if (processing_template_decl)
+    {
+      /* We should not have altered the return value.  */
+      gcc_assert (retval == saved_retval);
+      return retval;
+    }
+
   /* The fabled Named Return Value optimization, as per [class.copy]/15:
 
      [...]      For  a function with a class return type, if the expression
diff --git a/gcc/testsuite/g++.dg/template/pr18969-2.C b/gcc/testsuite/g++.dg/template/pr18969-2.C
new file mode 100644
index 0000000..e0b0c1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr18969-2.C
@@ -0,0 +1,11 @@ 
+// PR c++/18969
+// { dg-do compile { target c++14 } }
+
+template <typename T>
+struct A
+{
+    auto *f1 () { return; } // { dg-error "return-statement" }
+    auto &f2 () { return; } // { dg-error "return-statement" }
+
+    auto f3 () { return; } // { dg-bogus "return-statement" }
+};
diff --git a/gcc/testsuite/g++.dg/template/pr18969.C b/gcc/testsuite/g++.dg/template/pr18969.C
new file mode 100644
index 0000000..dba5eb9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr18969.C
@@ -0,0 +1,14 @@ 
+// PR c++/18969
+
+template <typename T>
+struct A
+{
+    int f1 () { return; } // { dg-error "return-statement" }
+    void f2 () { return 5; } // { dg-error "return-statement" }
+    T *f3 () { return; } // { dg-error "return-statement" }
+    typename T::f &f4 () { return; } // { dg-error "return-statement" }
+
+    T f5 () { return; } // { dg-bogus "return-statement" }
+    void f6 () { return (T)true; } // { dg-bogus "return-statement" }
+    typename T::f f7 () { return; } // { dg-bogus "return-statement" }
+};