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

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

[Patrick Palka]

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

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" }
+};
-- 
2.5.0.rc2.22.g69b1679.dirty

Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)

[Jason Merrill]

OK.

Jason

Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)

[Patrick Palka]

Committed with this additional change to fix a latent testcase bug:

diff --git a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
index 6a747ff..28b029f 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
@@ -5,7 +5,7 @@ enum bar {};
 void operator+ (int, int);// { dg-error "" } .*
 void operator+ (bar&, int);

-template <class T> void operator+ (int b, T& t) { return b; }
+template <class T> void operator+ (int b, T& t) { return; }
 void operator+ (int, bar&);

 template <class T> class foo

Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)

[Patrick Palka]

On Sun, Jul 26, 2015 at 1:09 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> Committed with this additional change to fix a latent testcase bug:
>
> diff --git a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
> b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
> index 6a747ff..28b029f 100644
> --- a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
> +++ b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
> @@ -5,7 +5,7 @@ enum bar {};
>  void operator+ (int, int);// { dg-error "" } .*
>  void operator+ (bar&, int);
>
> -template <class T> void operator+ (int b, T& t) { return b; }
> +template <class T> void operator+ (int b, T& t) { return; }
>  void operator+ (int, bar&);
>
>  template <class T> class foo

Hmm, on second thought, I don't think this fix is right.  It may be
the case that the 'return b;' was there to make instantiation of that
template a compile-time error. By changing it to 'return;'
instantiation is allowed.  Is this property important here?  Should I
preserve the original property (that instantiation is a compile-time
error) by instead doing the following?

    Adjust g++.old-deja/g++.jason/overload.C

    gcc/testsuite/ChangeLog:

        * g++.old-deja/g++.jason/overload.C: Adjust to preserve original
        property that instantiation is a compile-time error.

diff --git a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
index 28b029f..5d27713 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
@@ -5,7 +5,7 @@ enum bar {};
 void operator+ (int, int);// { dg-error "" } .*
 void operator+ (bar&, int);

-template <class T> void operator+ (int b, T& t) { return; }
+template <class T> void operator+ (int b, T& t) { (void) T::bogus; }
 void operator+ (int, bar&);

 template <class T> class foo

Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)

[Jason Merrill]

On 07/26/2015 01:50 PM, Patrick Palka wrote:
> Hmm, on second thought, I don't think this fix is right.  It may be
> the case that the 'return b;' was there to make instantiation of that
> template a compile-time error. By changing it to 'return;'
> instantiation is allowed.  Is this property important here?  Should I
> preserve the original property (that instantiation is a compile-time
> error) by instead doing the following?

Sure, can't hurt.

Jason