public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch] PR55189 enable -Wreturn-type by default
@ 2013-12-20 17:54 Sylvestre Ledru
  2013-12-27  5:26 ` Chung-Ju Wu
  2014-01-16 19:44 ` Jason Merrill
  0 siblings, 2 replies; 29+ messages in thread
From: Sylvestre Ledru @ 2013-12-20 17:54 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 5047 bytes --]

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


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 @@
 	      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;
 	    }





[-- Attachment #2: return-type-by-default-and-missing-return-test-suite.diff.gz --]
[-- Type: application/gzip, Size: 51266 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [Patch] PR55189 enable -Wreturn-type by default
@ 2014-06-04 19:49 Sylvestre Ledru
  2014-06-04 22:35 ` Mike Stump
  2014-06-04 23:31 ` Joseph S. Myers
  0 siblings, 2 replies; 29+ messages in thread
From: Sylvestre Ledru @ 2014-06-04 19:49 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1013 bytes --]

Hello,

Finally, I have been able to update all tests with -Wreturn-type enabled
by default. AFAIK, under GNU/Linux Debian Jessie 64 bits, there is no
PASS->FAIL tests.

Now, I would like to know if I can commit that into the repository. Who
can review that?

As attachment, you will find the actual (tiny) patch.

I split the tests update by languages. As they are big ( 1260 files
changed, 1638 insertions(+), 903 deletions(-) ), I uploaded the patches
on my server:
http://sylvestre.ledru.info/bordel/patch/0002-Update-Objective-c-tests-with-warning-return-type-en.patch
http://sylvestre.ledru.info/bordel/patch/0003-Update-Fortran-tests-with-warning-return-type-enable.patch
http://sylvestre.ledru.info/bordel/patch/0004-Update-gcc-tests-with-warning-return-type-enabled-by.patch
http://sylvestre.ledru.info/bordel/patch/0005-Update-C-tests-with-warning-return-type-enabled-by-d.patch
http://sylvestre.ledru.info/bordel/patch/0006-Update-OpenMP-tests-with-warning-return-type-enabled.patch

Thanks,
Sylvestre

[-- Attachment #2: 0001-Enable-warning-return-type-by-default.patch --]
[-- Type: text/x-patch, Size: 995 bytes --]

gcc/c-family/ChangeLog:

2014-06-04  Sylvestre Ledru  <sylvestre@debian.org>

	* c.opt: -Wreturn-type enabled by default

From 650edb9943ba8b2afb4995e70f671d8fdc26e10a Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Wed, 4 Jun 2014 13:33:40 +0200
Subject: [PATCH 1/6] Enable warning return-type by default

---
 gcc/c-family/c.opt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 5d36a80..8e78a9d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -686,7 +686,7 @@ C ObjC C++ ObjC++ Var(warn_return_local_addr) Init(1) Warning
 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++)
 
 Wselector
-- 
2.0.0


^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [Patch] PR55189 enable -Wreturn-type by default
@ 2014-08-14 18:49 Manuel López-Ibáñez
  2014-08-15 16:28 ` Sylvestre Ledru
  0 siblings, 1 reply; 29+ messages in thread
From: Manuel López-Ibáñez @ 2014-08-14 18:49 UTC (permalink / raw)
  To: Gcc Patch List; +Cc: Jason Merrill, Joseph S. Myers, Sylvestre Ledru

--- a/gcc/fortran/options.c
+++ b/gcc/fortran/options.c
@@ -693,6 +693,10 @@ gfc_handle_option (size_t scode, const char *arg,
int value,
       gfc_option.warn_line_truncation = value;
       break;

+    case OPT_Wmissing_return:
+      warn_missing_return = value;
+      break;
+
     case OPT_Wrealloc_lhs:
       gfc_option.warn_realloc_lhs = value;
       break;

The entry in c.opt says this is a C/C++ option, why you need this?

+Wmissing-return
+C ObjC C++ ObjC++ Var(warn_missing_return) LangEnabledBy(C ObjC C++
ObjC++,Wreturn-type)
+Warn whenever control may reach end of non-void function

This should prevent that using -Wreturn-type in Fortran tries to
enable -Wmissing-return, if not that is a bug.

In any case, the work-around should be adding Wmissing-return to
fortran/lang.opt with a ??? comment, not there.


--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -472,7 +472,7 @@ C ObjC Var(warn_implicit_function_declaration)
Init(-1) Warning LangEnabledBy(C
 Warn about implicit function declarations

 Wimplicit-int
-C ObjC Var(warn_implicit_int) Warning LangEnabledBy(C ObjC,Wimplicit)
+C ObjC Var(warn_implicit_int) Warning
 Warn when a declaration does not specify a type

 Wimport
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 5ae910c..3f2019a 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3615,7 +3615,7 @@ This warning is enabled by @option{-Wall} in C++.
 @opindex Wimplicit-int
 @opindex Wno-implicit-int
 Warn when a declaration does not specify a type.
-This warning is enabled by @option{-Wall}.
+This warning is enabled by default.

 @item -Wimplicit-function-declaration @r{(C and Objective-C only)}
 @opindex Wimplicit-function-declaration


Does this patch actually enables -Wimplicit-int by default? The
default without Init() should be zero!

And according to this: https://gcc.gnu.org/ml/gcc-patches/2014-06/msg01367.html

we still want -Wno-implicit to disable -Wimplicit-int (and
-Werror=implicit to set -Werror=implicit-int), so the LangEnabledBy()
should stay. The documentation could say: "This warning is enabled by
default and it is also controlled by -Wimplicit."

Cheers,

Manuel.

^ permalink raw reply	[flat|nested] 29+ messages in thread
end of thread, other threads:[~2014-08-20 21:58 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-20 17:54 [Patch] PR55189 enable -Wreturn-type by default Sylvestre Ledru
2013-12-27  5:26 ` Chung-Ju Wu
2013-12-27  5:32   ` Yury Gribov
2014-01-14 17:49     ` Sylvestre Ledru
2014-01-16 19:44 ` Jason Merrill
2014-01-23  6:44   ` Sylvestre Ledru
2014-01-23 18:48   ` Jason Merrill
2014-01-23 18:57     ` Sylvestre Ledru
2014-06-04 19:49 Sylvestre Ledru
2014-06-04 22:35 ` Mike Stump
2014-06-04 23:31 ` Joseph S. Myers
2014-06-05  9:33   ` Sylvestre Ledru
2014-06-05 18:01     ` Joseph S. Myers
2014-06-17 16:52       ` Sylvestre Ledru
2014-06-17 17:15         ` Joseph S. Myers
2014-06-17 17:37           ` Sylvestre Ledru
2014-06-17 17:41             ` Joseph S. Myers
2014-07-07 17:18               ` Sylvestre Ledru
2014-07-20 19:20                 ` Sylvestre Ledru
2014-07-30 22:10                 ` Joseph S. Myers
2014-08-11  7:44                   ` Sylvestre Ledru
2014-08-12 17:49                     ` Joseph S. Myers
2014-08-12 17:53                       ` Sylvestre Ledru
2014-08-14 17:01                       ` Sylvestre Ledru
2014-08-14 18:49 Manuel López-Ibáñez
2014-08-15 16:28 ` Sylvestre Ledru
2014-08-19 22:03   ` Joseph S. Myers
2014-08-20 21:42     ` Sylvestre Ledru
2014-08-20 21:58       ` Joseph S. Myers

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).