diff mbox

PR55189 enable -Wreturn-type by default

Message ID 52B4843D.3040804@debian.org
State New
Headers show

Commit Message

Sylvestre Ledru Dec. 20, 2013, 5:54 p.m. UTC
Hello

Following this thread http://gcc.gnu.org/ml/gcc/2013-11/msg00260.html
and this bug,
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55189

I would like to propose the two following patches:

I am activating -Wreturn-type by defaut and add the option -Wmissing-return

In Wreturn-type-by-default-testsuite.diff.gz (as a gziped attachment,
too big for this ML), I am updating all the tests (792). I understand
why nobody did it before, it is a few days of work and not really
fascinating. ;)

Basically, there were several cases:
1) Add return 0; (or return true;)
to make sure that the function returns something

2) Add -Wno-return-type to dg-options / dg-lto-options when it is too
hard to construct the type to return

3) explicit declaration of the function like:
-t()
+void t()

4) idem with main
-main() {
+int main() {

If there is a consensus on the fact that these patches can be applied, I
don't have any issue with signing the copyright assignment.

Thanks,
Sylvestre


 	      location = gimple_location (last);
 	      if (location == UNKNOWN_LOCATION)
 		  location = cfun->function_end_locus;
-	      warning_at (location, OPT_Wreturn_type, "control reaches end of
non-void function");
+	      warning_at (location, OPT_Wmissing_return, "control reaches end
of non-void function");
 	      TREE_NO_WARNING (cfun->decl) = 1;
 	      break;
 	    }

Comments

Chung-Ju Wu Dec. 27, 2013, 5:26 a.m. UTC | #1
2013/12/21 Sylvestre Ledru <sylvestre@debian.org>:
> Hello
>
> Following this thread http://gcc.gnu.org/ml/gcc/2013-11/msg00260.html
> and this bug,
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55189
>
> I would like to propose the two following patches:
>
> I am activating -Wreturn-type by defaut and add the option -Wmissing-return
>
[snip]
>
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog       (révision 206154)
> +++ gcc/ChangeLog       (copie de travail)
> @@ -1,3 +1,11 @@
> +2013-12-20  Sylvestre Ledru  <sylvestre@debian.org>
> +
> +        PR target/55189
> +        * -Wreturn-type enabled by default.
> +       * Introduce back the option -Wmissing-return (enabled by -Wall)
> +       It was included by default with -Wreturn-type
> +       * Update all tests failing because of these changes.
> +
>  2013-12-20  Eric Botcazou  <ebotcazou@adacore.com>
>         * config/arm/arm.c (arm_expand_prologue): In a nested APCS frame with

Hi, Sylvestre,

Sorry I have no right to approve this patch.
But I notice your ChangeLog formatting is not correct.

You can refer to other entries in ChangeLog to refine yours,
and then resubmit the patch for review. :)


Best regards,
jasonwucj
Yury Gribov Dec. 27, 2013, 5:32 a.m. UTC | #2
Chung-Wu wrote:
 > But I notice your ChangeLog formatting is not correct.
 >
 > You can refer to other entries in ChangeLog to refine yours,
 > and then resubmit the patch for review. :)

Or - use contrib/mklog to autogenerate template ChangeLog for you.

-Y
Jason Merrill Jan. 16, 2014, 7:44 p.m. UTC | #3
My preference would be to turn -Wreturn-type on by default, but not 
create the separate -Wmissing-return flag.  As I argued in 2002, there 
should only be one flag.

To avoid spurious warnings on code with infinite loops we could add a 
simple check for infinite loops and suppress the warning in that case. 
Basically, if we see a loop with an always-true condition and no breaks.

Jason
Sylvestre Ledru Jan. 23, 2014, 6:44 a.m. UTC | #4
On 16/01/2014 11:44, Jason Merrill wrote:
> My preference would be to turn -Wreturn-type on by default, but not
> create the separate -Wmissing-return flag.  As I argued in 2002, there
> should only be one flag.
I don't have any opinion on the subject. The separate option or not is
fine with me. I am just following an advice received :)

Sylvestre
diff mbox

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(révision 206154)
+++ gcc/ChangeLog	(copie de travail)
@@ -1,3 +1,11 @@ 
+2013-12-20  Sylvestre Ledru  <sylvestre@debian.org>
+
+        PR target/55189
+        * -Wreturn-type enabled by default.
+	* Introduce back the option -Wmissing-return (enabled by -Wall)
+	It was included by default with -Wreturn-type
+	* Update all tests failing because of these changes.
+
 2013-12-20  Eric Botcazou  <ebotcazou@adacore.com>
  	* config/arm/arm.c (arm_expand_prologue): In a nested APCS frame with
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(révision 206154)
+++ gcc/c-family/c.opt	(copie de travail)
@@ -673,9 +673,13 @@ 
 Warn about returning a pointer/reference to a local or temporary variable.
  Wreturn-type
-C ObjC C++ ObjC++ Var(warn_return_type) Warning LangEnabledBy(C ObjC
C++ ObjC++,Wall)
+C ObjC C++ ObjC++ Var(warn_return_type) Init(1) Warning
 Warn whenever a function's return type defaults to \"int\" (C), or
about inconsistent return types (C++)
 +Wmissing-return
+C ObjC C++ ObjC++ Var(warn_missing_return) Warning LangEnabledBy(C ObjC
C++ ObjC++,Wall)
+Warn whenever control may reach end of non-void function
+
 Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(révision 206154)
+++ gcc/doc/invoke.texi	(copie de travail)
@@ -261,7 +261,7 @@ 
 -Wparentheses  -Wpedantic-ms-format -Wno-pedantic-ms-format @gol
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls  -Wno-return-local-addr @gol
--Wreturn-type  -Wsequence-point  -Wshadow @gol
+-Wreturn-type -Wmissing-return  -Wsequence-point  -Wshadow @gol
 -Wsign-compare  -Wsign-conversion -Wfloat-conversion @gol
 -Wsizeof-pointer-memaccess @gol
 -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
@@ -3339,6 +3339,7 @@ 
 -Wpointer-sign  @gol
 -Wreorder   @gol
 -Wreturn-type  @gol
+-Wmissing-return  @gol
 -Wsequence-point  @gol
 -Wsign-compare @r{(only in C++)}  @gol
 -Wstrict-aliasing  @gol
@@ -3795,8 +3796,14 @@ 
 message, even when @option{-Wno-return-type} is specified.  The only
 exceptions are @samp{main} and functions defined in system headers.
 -This warning is enabled by @option{-Wall}.
+@item -Wmissing-return
+@opindex Wmissing-return
+@opindex Wno-missing-return
+Warn whenever falling off the end of the function body (I.e. without
+any return).
 +This warning is enabled by @option{-Wall} for C and C++.
+
 @item -Wswitch
 @opindex Wswitch
 @opindex Wno-switch
Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c	(révision 206154)
+++ gcc/fortran/options.c	(copie de travail)
@@ -717,6 +717,10 @@ 
       warn_return_type = value;
       break;
 +    case OPT_Wmissing_return:
+      warn_missing_return = value;
+      break;
+
     case OPT_Wsurprising:
       gfc_option.warn_surprising = value;
       break;
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(révision 206154)
+++ gcc/tree-cfg.c	(copie de travail)
@@ -8098,7 +8098,7 @@ 
    /* If we see "return;" in some basic block, then we do reach the end
      without returning a value.  */
-  else if (warn_return_type
+  else if (warn_missing_return
 	   && !TREE_NO_WARNING (cfun->decl)
 	   && EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds) > 0
 	   && !VOID_TYPE_P (TREE_TYPE (TREE_TYPE (cfun->decl))))
@@ -8113,7 +8113,7 @@