[patch] fix c++/27975, add new warning option -Wenum-compare

[patch] fix c++/27975, add new warning option -Wenum-compare

[Michael Matz]

Hi,

the bugreport asks for the possibility to disable the warning about 
comparison of values of different enum types.  The request seems 
reasonable and I've put it under the control of a new flag -Wenum-compare 
(only for C++ as only that frontend implements the warning).

Right now it's disabled by default, but we could enable it with -Wall or 
-Wextra, I'll do whatever is decided.

Regstrapping in progress, okay for trunk if that passes?


Ciao,
Michael.
	Fix c++/27975.

	* c.opt (Wenum-compare): New warning option.

cp/
	* call.c (build_new_op): Make warning conditional on OPT_Wenum_compare.

doc/
	* invoke.texi (Warning Options): Document -Wenum-compare.

Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 135599)
+++ gcc/c.opt	(working copy)
@@ -195,6 +195,10 @@ Wendif-labels
 C ObjC C++ ObjC++ Warning
 Warn about stray tokens after #elif and #endif
 
+Wenum-compare
+C++ ObjC++ Warning
+Warn about comparison of different enum types
+
 Werror
 C ObjC C++ ObjC++
 ; Documented in common.opt
Index: gcc/cp/call.c
===================================================================
*** gcc/cp/call.c	(revision 135599)
--- gcc/cp/call.c	(working copy)
*************** build_new_op (enum tree_code code, int f
*** 4004,4010 ****
  		      != TYPE_MAIN_VARIANT (TREE_TYPE (arg2)))
  		  && (complain & tf_warning))
  		{
! 		  warning (0, "comparison between %q#T and %q#T",
  			   TREE_TYPE (arg1), TREE_TYPE (arg2));
  		}
  	      break;
--- 4004,4011 ----
  		      != TYPE_MAIN_VARIANT (TREE_TYPE (arg2)))
  		  && (complain & tf_warning))
  		{
! 		  warning (OPT_Wenum_compare,
! 			   "comparison between %q#T and %q#T",
  			   TREE_TYPE (arg1), TREE_TYPE (arg2));
  		}
  	      break;
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 135599)
+++ gcc/doc/invoke.texi	(working copy)
@@ -232,7 +232,7 @@ Objective-C and Objective-C++ Dialects}.
 -Wchar-subscripts -Wclobbered  -Wcomment @gol
 -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
 -Wno-deprecated-declarations -Wdisabled-optimization  -Wno-div-by-zero  @gol
--Wempty-body  -Wno-endif-labels @gol
+-Wempty-body  -Wenum-compare -Wno-endif-labels @gol
 -Werror  -Werror=* @gol
 -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
 -Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
@@ -3663,6 +3664,11 @@ while} statement.  Additionally, in C++,
 in a @samp{while} or @samp{for} statement with no whitespacing before
 the semicolon.  This warning is also enabled by @option{-Wextra}.
 
+@item -Wenum-compare @r{(C++ and Objective-C++ only)}
+@opindex Wenum-compare
+@opindex Wno-enum-compare
+Warn about a comparison between values of different enum types.
+
 @item -Wsign-compare
 @opindex Wsign-compare
 @opindex Wno-sign-compare

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Ian Lance Taylor]

Michael Matz <matz@suse.de> writes:

> the bugreport asks for the possibility to disable the warning about 
> comparison of values of different enum types.  The request seems 
> reasonable and I've put it under the control of a new flag -Wenum-compare 
> (only for C++ as only that frontend implements the warning).
>
> Right now it's disabled by default, but we could enable it with -Wall or 
> -Wextra, I'll do whatever is decided.

It seems to me that this option was previously always emitted.  So I
think that -Wenum-compare should be turned on by -Wall.

Also it would be nice to have some test cases.  If you are lucky you
will find an existing test case which used to emit a warning and now
does not.

I'll say OK with -Wall turning on -Wenum-compare and with a test case,
but please give people a day to comment.

Thanks.

Ian

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Richard Guenther]

On Thu, May 22, 2008 at 12:05 AM, Ian Lance Taylor <iant@google.com> wrote:
> Michael Matz <matz@suse.de> writes:
>
>> the bugreport asks for the possibility to disable the warning about
>> comparison of values of different enum types.  The request seems
>> reasonable and I've put it under the control of a new flag -Wenum-compare
>> (only for C++ as only that frontend implements the warning).
>>
>> Right now it's disabled by default, but we could enable it with -Wall or
>> -Wextra, I'll do whatever is decided.
>
> It seems to me that this option was previously always emitted.  So I
> think that -Wenum-compare should be turned on by -Wall.

IMHO it should be on by default as an implicit conversion of one enum
type to another
is an error in C++.

Richard.

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Ian Lance Taylor]

"Richard Guenther" <richard.guenther@gmail.com> writes:

> On Thu, May 22, 2008 at 12:05 AM, Ian Lance Taylor <iant@google.com> wrote:
>> Michael Matz <matz@suse.de> writes:
>>
>>> the bugreport asks for the possibility to disable the warning about
>>> comparison of values of different enum types.  The request seems
>>> reasonable and I've put it under the control of a new flag -Wenum-compare
>>> (only for C++ as only that frontend implements the warning).
>>>
>>> Right now it's disabled by default, but we could enable it with -Wall or
>>> -Wextra, I'll do whatever is decided.
>>
>> It seems to me that this option was previously always emitted.  So I
>> think that -Wenum-compare should be turned on by -Wall.
>
> IMHO it should be on by default as an implicit conversion of one enum
> type to another
> is an error in C++.

Fine with me.

Ian

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Mark Mitchell]

Ian Lance Taylor wrote:

>>>> Right now it's disabled by default, but we could enable it with -Wall or
>>>> -Wextra, I'll do whatever is decided.
>>> It seems to me that this option was previously always emitted.  So I
>>> think that -Wenum-compare should be turned on by -Wall.
>> IMHO it should be on by default as an implicit conversion of one enum
>> type to another
>> is an error in C++.
> 
> Fine with me.

Agreed.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Michael Matz]

Hi,

On Thu, 22 May 2008, Richard Guenther wrote:

> >> Right now it's disabled by default, but we could enable it with -Wall or
> >> -Wextra, I'll do whatever is decided.
> >
> > It seems to me that this option was previously always emitted.  So I
> > think that -Wenum-compare should be turned on by -Wall.
> 
> IMHO it should be on by default as an implicit conversion of one enum 
> type to another is an error in C++.

I've checked in the below as r136035.  It adds two testcases (there were 
none for this warning), and activates the warning by default without any 
options, as before.  (all regstrapped on i686 and x86-64)


Ciao,
Michael.
-- 
	Fix c++/27975.

	* c.opt (Wenum-compare): New warning option.
	* doc/invoke.texi (Warning Options): Document -Wenum-compare.

cp/
	* call.c (build_new_op): Make warning conditional on OPT_Wenum_compare.

testsuite/
	* g++.dg/warn/Wenum-compare.C: New testcase.
	* g++.dg/warn/Wenum-compare-no.C: Ditto.

Index: c.opt
===================================================================
*** c.opt	(revision 136032)
--- c.opt	(working copy)
*************** Wendif-labels
*** 195,200 ****
--- 195,204 ----
  C ObjC C++ ObjC++ Warning
  Warn about stray tokens after #elif and #endif
  
+ Wenum-compare
+ C++ ObjC++ Var(warn_enum_compare) Init(1) Warning
+ Warn about comparison of different enum types
+ 
  Werror
  C ObjC C++ ObjC++
  ; Documented in common.opt
Index: cp/call.c
===================================================================
*** cp/call.c	(revision 136034)
--- cp/call.c	(working copy)
*************** build_new_op (enum tree_code code, int f
*** 4004,4010 ****
  		      != TYPE_MAIN_VARIANT (TREE_TYPE (arg2)))
  		  && (complain & tf_warning))
  		{
! 		  warning (0, "comparison between %q#T and %q#T",
  			   TREE_TYPE (arg1), TREE_TYPE (arg2));
  		}
  	      break;
--- 4004,4011 ----
  		      != TYPE_MAIN_VARIANT (TREE_TYPE (arg2)))
  		  && (complain & tf_warning))
  		{
! 		  warning (OPT_Wenum_compare,
! 			   "comparison between %q#T and %q#T",
  			   TREE_TYPE (arg1), TREE_TYPE (arg2));
  		}
  	      break;
Index: doc/invoke.texi
===================================================================
*** doc/invoke.texi	(revision 135970)
--- doc/invoke.texi	(working copy)
*************** Objective-C and Objective-C++ Dialects}.
*** 232,238 ****
  -Wchar-subscripts -Wclobbered  -Wcomment @gol
  -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
  -Wno-deprecated-declarations -Wdisabled-optimization  -Wno-div-by-zero  @gol
! -Wempty-body  -Wno-endif-labels @gol
  -Werror  -Werror=* @gol
  -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
  -Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
--- 232,238 ----
  -Wchar-subscripts -Wclobbered  -Wcomment @gol
  -Wconversion  -Wcoverage-mismatch  -Wno-deprecated  @gol
  -Wno-deprecated-declarations -Wdisabled-optimization  -Wno-div-by-zero  @gol
! -Wempty-body  -Wenum-compare -Wno-endif-labels @gol
  -Werror  -Werror=* @gol
  -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
  -Wno-format-contains-nul -Wno-format-extra-args -Wformat-nonliteral @gol
*************** while} statement.  Additionally, in C++,
*** 3658,3663 ****
--- 3659,3669 ----
  in a @samp{while} or @samp{for} statement with no whitespacing before
  the semicolon.  This warning is also enabled by @option{-Wextra}.
  
+ @item -Wenum-compare @r{(C++ and Objective-C++ only)}
+ @opindex Wenum-compare
+ @opindex Wno-enum-compare
+ Warn about a comparison between values of different enum types.
+ 
  @item -Wsign-compare
  @opindex Wsign-compare
  @opindex Wno-sign-compare
Index: testsuite/g++.dg/warn/Wenum-compare-no.C
===================================================================
*** testsuite/g++.dg/warn/Wenum-compare-no.C	(revision 0)
--- testsuite/g++.dg/warn/Wenum-compare-no.C	(revision 0)
***************
*** 0 ****
--- 1,10 ----
+ /* Test disabling -Wenum-compare (on by default).  See PR27975.  */
+ /* { dg-do compile } */
+ /* { dg-options "-Wno-enum-compare" } */
+ enum E1 { a };
+ enum E2 { b };
+ 
+ int foo (E1 e1, E2 e2)
+ {
+   return e1 == e2;  /* { dg-bogus "comparison between" } */
+ }
Index: testsuite/g++.dg/warn/Wenum-compare.C
===================================================================
*** testsuite/g++.dg/warn/Wenum-compare.C	(revision 0)
--- testsuite/g++.dg/warn/Wenum-compare.C	(revision 0)
***************
*** 0 ****
--- 1,10 ----
+ /* Test that we get the -Wenum-compare by default.  See PR27975.  */
+ /* { dg-do compile } */
+ /* { dg-options "" } */
+ enum E1 { a };
+ enum E2 { b };
+ 
+ int foo (E1 e1, E2 e2)
+ {
+   return e1 == e2;  /* { dg-warning "comparison between" } */
+ }

Re: [patch] fix c++/27975, add new warning option -Wenum-compare

[Manuel López-Ibáñez]

Please, add a line to http://gcc.gnu.org/gcc-4.4/changes.html
mentioning the new option.

We tried to report new options in
http://gcc.gnu.org/gcc-4.3/changes.html, so let's keep doing it.

Thanks,

Manuel.

2008/5/27 Michael Matz <matz@suse.de>:
>        * c.opt (Wenum-compare): New warning option.