* [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)
@ 2015-07-26 3:47 Patrick Palka
2015-07-26 8:40 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2015-07-26 3:47 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, lopezibanez, 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)
2015-07-26 3:47 [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late) Patrick Palka
@ 2015-07-26 8:40 ` Jason Merrill
2015-07-26 17:50 ` Patrick Palka
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2015-07-26 8:40 UTC (permalink / raw)
To: Patrick Palka, gcc-patches; +Cc: lopezibanez
OK.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)
2015-07-26 8:40 ` Jason Merrill
@ 2015-07-26 17:50 ` Patrick Palka
2015-07-26 19:10 ` Patrick Palka
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2015-07-26 17:50 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Manuel López-Ibáñez
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)
2015-07-26 17:50 ` Patrick Palka
@ 2015-07-26 19:10 ` Patrick Palka
2015-08-20 22:53 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Patrick Palka @ 2015-07-26 19:10 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches, Manuel López-Ibáñez
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late)
2015-07-26 19:10 ` Patrick Palka
@ 2015-08-20 22:53 ` Jason Merrill
0 siblings, 0 replies; 5+ messages in thread
From: Jason Merrill @ 2015-08-20 22:53 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches, Manuel López-Ibáñez
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-20 22:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-26 3:47 [PATCH] Fix PR c++/18969 (invalid return statement diagnosed too late) Patrick Palka
2015-07-26 8:40 ` Jason Merrill
2015-07-26 17:50 ` Patrick Palka
2015-07-26 19:10 ` Patrick Palka
2015-08-20 22:53 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).