Patchwork RFA: fix PR c/48116

login
register
mail settings
Submitter Tom Tromey
Date March 16, 2011, 5:18 p.m.
Message ID <m339mn3rxv.fsf@fleche.redhat.com>
Download mbox | patch
Permalink /patch/87285/
State New
Headers show

Comments

Tom Tromey - March 16, 2011, 5:18 p.m.
This patch fixes PR c/48116.

The bug is that -Wreturn-type does not follow the documentation.  In
particular, it should warn for this code, but does not:

    static void f() {}
    static void g() { return f(); }

I think the bug is that c-typeck.c calls pedwarn with either 0 or
OPT_pedantic, but it should use OPT_Wreturn_type in some cases.

I am not completely sure this is the correct patch.  In particular, this
patch chooses to report -Wreturn-type over -pedantic, e.g.:

    opsy. gcc -pedantic -Wreturn-type --syntax-only q.c
    q.c: In function ‘y’:
    q.c:3:16: warning: ISO C forbids ‘return’ with expression, in function returning void [-Wreturn-type]

This is a somewhat odd situation, in that both -Wreturn-type and
-pedantic must be disabled to eliminate this message.

Also, arguably there should be a different message when -pedantic is not
given.

Bootstrapped and regtested on x86-64 (compile farm).
New test case included.

I did not check to see whether this is a regression.

Tom

2011-03-16  Tom Tromey  <tromey@redhat.com>

	PR c/48116
	* c-typeck.c (c_finish_return): Check warn_return_type.

2011-03-16  Tom Tromey  <tromey@redhat.com>

	* gcc.dg/Wreturn-type3.c: New file.
Jakub Jelinek - March 16, 2011, 5:28 p.m.
On Wed, Mar 16, 2011 at 11:18:04AM -0600, Tom Tromey wrote:
> This patch fixes PR c/48116.
> 
> The bug is that -Wreturn-type does not follow the documentation.  In
> particular, it should warn for this code, but does not:
> 
>     static void f() {}
>     static void g() { return f(); }
> 
> I think the bug is that c-typeck.c calls pedwarn with either 0 or
> OPT_pedantic, but it should use OPT_Wreturn_type in some cases.

I think it is wrong if -Wreturn-type, which is enabled by default in -Wall,
warns about the above.  Returning a void value in a void function is a GNU
extension, which is heavily used in various projects.
Warning about this just with -pedantic is IMHO the right thing to do.

	Jakub
Tom Tromey - March 16, 2011, 7:59 p.m.
Tom> This patch fixes PR c/48116.

Tom> The bug is that -Wreturn-type does not follow the documentation.  In
Tom> particular, it should warn for this code, but does not:

Tom> static void f() {}
Tom> static void g() { return f(); }

Tom> I think the bug is that c-typeck.c calls pedwarn with either 0 or
Tom> OPT_pedantic, but it should use OPT_Wreturn_type in some cases.

Jakub> I think it is wrong if -Wreturn-type, which is enabled by default
Jakub> in -Wall, warns about the above.  Returning a void value in a
Jakub> void function is a GNU extension, which is heavily used in
Jakub> various projects.  Warning about this just with -pedantic is IMHO
Jakub> the right thing to do.

It seems ok to me to accept it as a GNU extension.
But, it still doesn't warn with -std=c89 or -std=c99.
-pedantic seems too heavy for this, to me.

If you agree I will look at fixing the patch.
Otherwise I guess I will write a documentation patch to remove this
text, since it is just incorrect.

Tom
Joseph S. Myers - March 18, 2011, 6:19 p.m.
On Wed, 16 Mar 2011, Tom Tromey wrote:

> It seems ok to me to accept it as a GNU extension.
> But, it still doesn't warn with -std=c89 or -std=c99.
> -pedantic seems too heavy for this, to me.

It is not the function of -std to enable diagnostics for extensions; 
that's the function of -pedantic.  This (returning void expressions from a 
function returning void) is an extension that's standard in C++; it should 
probably be documented as an extension in GNU C if it isn't already 
documented.

Patch

Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 170953)
+++ c-typeck.c	(working copy)
@@ -8628,10 +8628,11 @@ 
     {
       current_function_returns_null = 1;
       if (TREE_CODE (TREE_TYPE (retval)) != VOID_TYPE)
-	pedwarn (loc, 0,
+	pedwarn (loc, warn_return_type ? OPT_Wreturn_type : 0,
 		 "%<return%> with a value, in function returning void");
       else
-	pedwarn (loc, OPT_pedantic, "ISO C forbids "
+	pedwarn (loc, warn_return_type ? OPT_Wreturn_type : OPT_pedantic,
+		 "ISO C forbids "
 		 "%<return%> with expression, in function returning void");
     }
   else
Index: testsuite/gcc.dg/Wreturn-type3.c
===================================================================
--- testsuite/gcc.dg/Wreturn-type3.c	(revision 0)
+++ testsuite/gcc.dg/Wreturn-type3.c	(revision 0)
@@ -0,0 +1,6 @@ 
+/* PR c/48116 */
+/* { dg-do compile } */
+/* { dg-options "-Wreturn-type" } */
+
+static void f() {}
+static void g() { return f(); }	/* { dg-warning "forbids .return" "missing return" } */