diff mbox

[FIX,PR,c/48116] -Wreturn-type does not work as advertised

Message ID CAE+uiWYwGEeAHs7LiTaxyQZgkTdJ7Y1Pvzaf8pVi0WxbBBJv7A@mail.gmail.com
State New
Headers show

Commit Message

Prasad Ghangal April 11, 2016, 2:39 p.m. UTC
Hi!

This is proposed patch for
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48116 (-Wreturn-type does
not work as advertised)

Currently gcc doesn't give any warning with -Wreturn-type or -Wall
option for test cases like :

void x (void) { }
void y(void) { return x(); }


applying this patch gives:

$gcc foo.c -S -Wreturn-type
foo.c: In function ‘y’:
foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
function returning void [-Wreturn-type]
 void y(void) { return x(); }
                       ^~~
foo.c:2:6: note: declared here
 void y(void) { return x(); }
      ^

$gcc foo.c -S -Wall
foo.c: In function ‘y’:
foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
function returning void [-Wreturn-type]
 void y(void) { return x(); }
                       ^~~
foo.c:2:6: note: declared here
 void y(void) { return x(); }
      ^

$gcc foo.c -S -pedantic
foo.c: In function ‘y’:
foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
function returning void [-Wpedantic]
 void y(void) { return x(); }
                       ^~~
foo.c:2:6: note: declared here
 void y(void) { return x(); }
      ^


I have fully bootstrapped and tested on x86_64-pc-linux.



Thanks,
Prasad Ghangal

Comments

Prasad Ghangal April 25, 2016, 8:05 p.m. UTC | #1
On 11 April 2016 at 20:09, Prasad Ghangal <prasad.ghangal@gmail.com> wrote:
>
> Hi!
>
> This is proposed patch for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48116 (-Wreturn-type does
> not work as advertised)
>
> Currently gcc doesn't give any warning with -Wreturn-type or -Wall
> option for test cases like :
>
> void x (void) { }
> void y(void) { return x(); }
>
>
> applying this patch gives:
>
> $gcc foo.c -S -Wreturn-type
> foo.c: In function ‘y’:
> foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
> function returning void [-Wreturn-type]
>  void y(void) { return x(); }
>                        ^~~
> foo.c:2:6: note: declared here
>  void y(void) { return x(); }
>       ^
>
> $gcc foo.c -S -Wall
> foo.c: In function ‘y’:
> foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
> function returning void [-Wreturn-type]
>  void y(void) { return x(); }
>                        ^~~
> foo.c:2:6: note: declared here
>  void y(void) { return x(); }
>       ^
>
> $gcc foo.c -S -pedantic
> foo.c: In function ‘y’:
> foo.c:2:23: warning: ISO C forbids ‘return’ with expression, in
> function returning void [-Wpedantic]
>  void y(void) { return x(); }
>                        ^~~
> foo.c:2:6: note: declared here
>  void y(void) { return x(); }
>       ^
>
>
> I have fully bootstrapped and tested on x86_64-pc-linux.
>
>
>
> Thanks,
> Prasad Ghangal



*PING*

https://gcc.gnu.org/ml/gcc-patches/2016-04/msg00474.html

Is this patch OK for gcc 7?


Thanks,
Prasad Ghangal
Joseph Myers May 4, 2016, 4:02 p.m. UTC | #2
On Mon, 11 Apr 2016, Prasad Ghangal wrote:

> Hi!
> 
> This is proposed patch for
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48116 (-Wreturn-type does
> not work as advertised)

I think that this is actually a documentation bug and should be fixed by 
changing the documentation, not by changing the code.  That is, the patch 
<https://gcc.gnu.org/ml/gcc-patches/2007-02/msg01102.html> that added the 
assertion about return with a value from functions returning void was 
mistaken: if the expression is non-void, the diagnostic is mandatory, and 
if it's a void expression, this is a legitimate, long-standing extension 
to ISO C, taken from ISO C++, and should be documented as such and not 
warned for except with -pedantic.  So there should be two documentation 
changes: (a) correct the description of -Wreturn-type and (b) explicitly 
document return of void expressions from functions returning void as a GNU 
C extension taken from ISO C++.
diff mbox

Patch

Index: gcc/c/ChangeLog
===================================================================
diff --git a/trunk/gcc/c/ChangeLog b/trunk/gcc/c/ChangeLog
--- a/trunk/gcc/c/ChangeLog	(revision 234872)
+++ b/trunk/gcc/c/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2016-04-11  Prasad Ghangal  <prasad.ghangal@gmail.com>
+
+	PR c/48116
+	* c-typeck.c (c_finish_return): Check -Wreturn-type option
+
 2016-04-04  Marek Polacek  <polacek@redhat.com>
 
 	PR c/70307
Index: gcc/c/c-typeck.c
===================================================================
diff --git a/trunk/gcc/c/c-typeck.c b/trunk/gcc/c/c-typeck.c
--- a/trunk/gcc/c/c-typeck.c	(revision 234872)
+++ b/trunk/gcc/c/c-typeck.c	(working copy)
@@ -9680,6 +9680,10 @@ 
 	warned_here = pedwarn
 	  (xloc, 0,
 	   "%<return%> with a value, in function returning void");
+      else if (warn_return_type)
+	warned_here = warning_at
+	  (xloc, OPT_Wreturn_type, "ISO C forbids "
+	   "%<return%> with expression, in function returning void");
       else
 	warned_here = pedwarn
 	  (xloc, OPT_Wpedantic, "ISO C forbids "
Index: gcc/testsuite/ChangeLog
===================================================================
diff --git a/trunk/gcc/testsuite/ChangeLog b/trunk/gcc/testsuite/ChangeLog
--- a/trunk/gcc/testsuite/ChangeLog	(revision 234872)
+++ b/trunk/gcc/testsuite/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2016-04-11  Prasad Ghangal  <prasad.ghangal@gmail.com>
+
+	PR c/48116
+	* gcc.dg/pr48116.c: New test
+
 2016-04-10  Paolo Carlini  <paolo.carlini@oracle.com>
 
 	PR c++/69066
Index: gcc/testsuite/gcc.dg/pr48116.c
===================================================================
diff --git a/trunk/gcc/testsuite/gcc.dg/pr48116.c b/trunk/gcc/testsuite/gcc.dg/pr48116.c
new file mode 10644
--- /dev/null	(revision 0)
+++ b/trunk/gcc/testsuite/gcc.dg/pr48116.c	(working copy)
@@ -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" } */
Index: libatomic/ChangeLog
===================================================================
diff --git a/trunk/libatomic/ChangeLog b/trunk/libatomic/ChangeLog
--- a/trunk/libatomic/ChangeLog	(revision 234872)
+++ b/trunk/libatomic/ChangeLog	(working copy)
@@ -1,3 +1,8 @@ 
+2016-04-11  Prasad Ghangal  <prasad.ghangal@gmail.com>
+
+	PR c/48116
+	* flag.c (atomic_flag_clear_explicit): Remove return 
+
 2016-01-06  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
 	PR other/67627
Index: libatomic/flag.c
===================================================================
diff --git a/trunk/libatomic/flag.c b/trunk/libatomic/flag.c
--- a/trunk/libatomic/flag.c	(revision 234872)
+++ b/trunk/libatomic/flag.c	(working copy)
@@ -60,5 +60,5 @@ 
 (atomic_flag_clear_explicit) (volatile atomic_flag *object,
 			      memory_order order)
 {
-  return atomic_flag_clear_explicit (object, order);
+  atomic_flag_clear_explicit (object, order);
 }
Index: libgomp/ChangeLog
===================================================================
diff --git a/trunk/libgomp/ChangeLog b/trunk/libgomp/ChangeLog
--- a/trunk/libgomp/ChangeLog	(revision 234872)
+++ b/trunk/libgomp/ChangeLog	(working copy)
@@ -1,3 +1,12 @@ 
+2016-04-11  Prasad Ghangal  <prasad.ghangal@gmail.com>
+	
+	PR c/48116
+	* fortran.c (omp_set_default_device_): Remove return 
+	* fortran.c (omp_set_default_device_8_): Likewise
+	* target.c (GOMP_target): Split return and call on separate lines
+	* target.c (GOMP_target_data): Likewise
+	* target.c (GOMP_taget_data_ext): Likewise
+
 2016-04-08  Cesar Philippidis  <cesar@codesourcery.com>
 
 	PR lto/70289
Index: libgomp/fortran.c
===================================================================
diff --git a/trunk/libgomp/fortran.c b/trunk/libgomp/fortran.c
--- a/trunk/libgomp/fortran.c	(revision 234872)
+++ b/trunk/libgomp/fortran.c	(working copy)
@@ -526,13 +526,13 @@ 
 void
 omp_set_default_device_ (const int32_t *device_num)
 {
-  return omp_set_default_device (*device_num);
+  omp_set_default_device (*device_num);
 }
 
 void
 omp_set_default_device_8_ (const int64_t *device_num)
 {
-  return omp_set_default_device (TO_INT (*device_num));
+  omp_set_default_device (TO_INT (*device_num));
 }
 
 int32_t
Index: libgomp/target.c
===================================================================
diff --git a/trunk/libgomp/target.c b/trunk/libgomp/target.c
--- a/trunk/libgomp/target.c	(revision 234872)
+++ b/trunk/libgomp/target.c	(working copy)
@@ -1465,7 +1465,10 @@ 
       /* All shared memory devices should use the GOMP_target_ext function.  */
       || devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM
       || !(fn_addr = gomp_get_target_fn_addr (devicep, fn)))
-    return gomp_target_fallback (fn, hostaddrs);
+    {
+      gomp_target_fallback (fn, hostaddrs);
+      return;
+    }
 
   struct target_mem_desc *tgt_vars
     = gomp_map_vars (devicep, mapnum, hostaddrs, NULL, sizes, kinds, false,
@@ -1616,7 +1619,10 @@ 
   if (devicep == NULL
       || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
       || (devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM))
-    return gomp_target_data_fallback ();
+    {
+      gomp_target_data_fallback ();
+      return;
+    }
 
   struct target_mem_desc *tgt
     = gomp_map_vars (devicep, mapnum, hostaddrs, NULL, sizes, kinds, false,
@@ -1635,7 +1641,10 @@ 
   if (devicep == NULL
       || !(devicep->capabilities & GOMP_OFFLOAD_CAP_OPENMP_400)
       || devicep->capabilities & GOMP_OFFLOAD_CAP_SHARED_MEM)
-    return gomp_target_data_fallback ();
+    {
+      gomp_target_data_fallback ();
+      return;
+    }
 
   struct target_mem_desc *tgt
     = gomp_map_vars (devicep, mapnum, hostaddrs, NULL, sizes, kinds, true,