diff mbox

Warn about return with a void expression with -Wreturn-type.

Message ID 20160601115504.725560-2-marbacz@gmail.com
State New
Headers show

Commit Message

Marcin Baczyński June 1, 2016, 11:55 a.m. UTC
PR c/48116.

Botstrapped and tested on x86_64-pc-linux-gnu.

gcc/ChangeLog:

   * c/c-typeck.c (c_finish_return): emit warning about return with a
    void expression in a function returning void if warn_return_type.

gcc/testsuite/ChangeLog:

   * gcc.dg/Wreturn-type3.c: new test.
---
 gcc/c/c-typeck.c                     | 4 ++++
 gcc/testsuite/gcc.dg/Wreturn-type3.c | 6 ++++++
 2 files changed, 10 insertions(+)
 create mode 100644 gcc/testsuite/gcc.dg/Wreturn-type3.c

Comments

Jakub Jelinek June 1, 2016, 12:07 p.m. UTC | #1
On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote:
> PR c/48116.
> 
> Botstrapped and tested on x86_64-pc-linux-gnu.
> 
> gcc/ChangeLog:
> 
>    * c/c-typeck.c (c_finish_return): emit warning about return with a
>     void expression in a function returning void if warn_return_type.

This is a GNU extension, so I fail to see why you should warn.  Furthermore,
the way you've done it, you would still warn instead of emit error e.g. with
-pedantic-errors on it.
Also, c/ prefixes don't belong to ChangeLog entries, gcc/c/ChangeLog is
-separate, and after : the first letter should be capitalized.
> 
> gcc/testsuite/ChangeLog:
> 
>    * gcc.dg/Wreturn-type3.c: new test.
> ---
>  gcc/c/c-typeck.c                     | 4 ++++
>  gcc/testsuite/gcc.dg/Wreturn-type3.c | 6 ++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/Wreturn-type3.c
> 
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 1520c20..8ac6cd4 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -9700,6 +9700,10 @@ c_finish_return (location_t loc, tree retval, tree origtype)
>  	warned_here = pedwarn
>  	  (xloc, 0,
>  	   "%<return%> with a value, in function returning void");
> +      else if (warn_return_type)
> +	warned_here = warning_at
> +	  (loc, OPT_Wreturn_type,
> +	   "%<return%> with expression, in function returning void");
>        else
>  	warned_here = pedwarn
>  	  (xloc, OPT_Wpedantic, "ISO C forbids "
> diff --git a/gcc/testsuite/gcc.dg/Wreturn-type3.c b/gcc/testsuite/gcc.dg/Wreturn-type3.c
> new file mode 100644
> index 0000000..e83c94b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wreturn-type3.c
> @@ -0,0 +1,6 @@
> +/* PR c/48116 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wreturn-type" } */
> +
> +void a() {}
> +void b() { return a(); }  /* { dg-warning "return" "returning void" } */
> -- 
> 2.8.3

	Jakub
Jakub Jelinek June 1, 2016, 4:39 p.m. UTC | #2
On Wed, Jun 01, 2016 at 02:44:22PM +0200, Marcin Baczyński wrote:
> On 1 Jun 2016 14:07, "Jakub Jelinek" <jakub@redhat.com> wrote:
> >
> > On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote:
> > > PR c/48116.
> > >
> > > Botstrapped and tested on x86_64-pc-linux-gnu.
> > >
> > > gcc/ChangeLog:
> > >
> > >    * c/c-typeck.c (c_finish_return): emit warning about return with a
> > >     void expression in a function returning void if warn_return_type.
> >
> > This is a GNU extension, so I fail to see why you should warn.
> 
> The bug is on the easyhacks list, I thought it would be a good place to
> start digging into the GCC code base.

First of all, I'm not the C FE maintainer, so these are just IMHO comments,
the maintainers might have different opinion.

> Do you have any suggestions for better things to start with?

Welcome to GCC hacking!  I'm must say I don't know in what state the easy
hacks list is, but for bugs that have been sitting in bugzilla for many
years there often is a reason why they haven't been addressed - it could be
just lack of interest from anybody, or that they fell through without
anybody noticing, but could be also that they aren't all that easy.
Often the easiest to fix bugs could be among the most recently filed PRs.

	Jakub
Martin Sebor June 1, 2016, 5:03 p.m. UTC | #3
On 06/01/2016 10:39 AM, Jakub Jelinek wrote:
> On Wed, Jun 01, 2016 at 02:44:22PM +0200, Marcin Baczyński wrote:
>> On 1 Jun 2016 14:07, "Jakub Jelinek" <jakub@redhat.com> wrote:
>>>
>>> On Wed, Jun 01, 2016 at 01:55:04PM +0200, Marcin Baczyński wrote:
>>>> PR c/48116.
>>>>
>>>> Botstrapped and tested on x86_64-pc-linux-gnu.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * c/c-typeck.c (c_finish_return): emit warning about return with a
>>>>      void expression in a function returning void if warn_return_type.
>>>
>>> This is a GNU extension, so I fail to see why you should warn.
>>
>> The bug is on the easyhacks list, I thought it would be a good place to
>> start digging into the GCC code base.
>
> First of all, I'm not the C FE maintainer, so these are just IMHO comments,
> the maintainers might have different opinion.
>
>> Do you have any suggestions for better things to start with?

Since you've done a lot of work on this bug already I think it
would be worthwhile to see it through to completion.  Given that,
as Jakub mentioned, the absence of the warning in permissive mode
(i.e. without -Wpedantic) is a GCC extension to C, it would be
helpful to mention it in the manual (doing so would resolve the
submitter's complaint that the -Wreturn-type option doesn't work
as documented).  With the documentation patch accepted, the bug
can then be closed as resolved/fixed and taken off the easy hacks
list.

Martin
diff mbox

Patch

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 1520c20..8ac6cd4 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -9700,6 +9700,10 @@  c_finish_return (location_t loc, tree retval, tree origtype)
 	warned_here = pedwarn
 	  (xloc, 0,
 	   "%<return%> with a value, in function returning void");
+      else if (warn_return_type)
+	warned_here = warning_at
+	  (loc, OPT_Wreturn_type,
+	   "%<return%> with expression, in function returning void");
       else
 	warned_here = pedwarn
 	  (xloc, OPT_Wpedantic, "ISO C forbids "
diff --git a/gcc/testsuite/gcc.dg/Wreturn-type3.c b/gcc/testsuite/gcc.dg/Wreturn-type3.c
new file mode 100644
index 0000000..e83c94b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wreturn-type3.c
@@ -0,0 +1,6 @@ 
+/* PR c/48116 */
+/* { dg-do compile } */
+/* { dg-options "-Wreturn-type" } */
+
+void a() {}
+void b() { return a(); }  /* { dg-warning "return" "returning void" } */