Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

[Manuel López-Ibáñez]

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

:ADDPATCH c++:

This patch continues the effort to fix PR7651 [1].

A new option -Wenum-promotion takes over the warning for an enumerator
and a non-enumerator appearing in a conditional expression. The new
option is enabled by -Wextra, so we keep the current behaviour but add
the ability to enable/disable this individual warning.

Notice that there is an unnamed and unconditional warning about
different enumeration types used in conditional expression (see
cp/call.c in the patch). That warning is untouched. This patch only
gives a name to the warning that is enabled by Wextra.

Bootstrapped and regression tested with --enable-languages=all on
i686-pc-linux-gnu.

OK for mainline?

[1] http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01103.html

2007-01-07  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  PR middle-end/7651
   * c.opt (Wenum-promotion): New.
   * doc/invoke.texi (Wenum-promotion): Document it.
   (Wextra): Enabled by -Wextra.
   (C++ Language Options): Add it.
   * c-opts.c (c_common_post_options): Enabled by -Wextra.

cp/
2007-01-07  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  PR middle-end/7651
  * call.c (build_conditional_expr): Replace Wextra with Wenum-promotion.

testsuite/
2007-01-07  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>

  PR middle-end/7651
  * g++.old-deja/g++.other/cond5.C: Move enumerators testcases to...
  * g++.dg/warn/Wenum-promotion.C: ... here. New.
  * g++.dg/warn/Wenum-promotion-no.C: ... here. New.
  * g++.dg/warn/Wenum-promotion-Wextra.C: ... here. New.

[-- Attachment #2: wenum-promotion.diff --]
[-- Type: text/plain, Size: 8337 bytes --]

Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 120347)
+++ gcc/doc/invoke.texi	(working copy)
@@ -192,7 +192,7 @@ in the following sections.
 -Weffc++  -Wno-deprecated  -Wstrict-null-sentinel @gol
 -Wno-non-template-friend  -Wold-style-cast @gol
 -Woverloaded-virtual  -Wno-pmf-conversions @gol
--Wsign-promo}
+-Wsign-promo  -Wenum-promotion}
 
 @item Objective-C and Objective-C++ Language Options
 @xref{Objective-C and Objective-C++ Dialect Options,,Options Controlling
@@ -2934,6 +2934,7 @@ overflow, underflow, loss of precision, 
 
 @item @r{(C++ only)}
 An enumerator and a non-enumerator both appear in a conditional expression.
+This warning can be independently controlled by @option{-Wenum-promotion}.
 
 @item @r{(C++ only)}
 A non-static reference or non-static @samp{const} member appears in a
@@ -3179,6 +3180,12 @@ changed by the conversion like in @code{
 An empty body occurs in an @samp{if} or @samp{else} statement. 
 This warning is also enabled by @option{-Wextra}.
 
+@item -Wenum-promotion @r{(C++ only)}
+@opindex Wenum-promotion
+Warn if an enumerated type and a non-enumerated type both appear in a
+conditional expression.  This warning is also enabled by
+@option{-Wextra}.
+
 @item -Wsign-compare
 @opindex Wsign-compare
 @cindex warning for comparison of signed and unsigned values
Index: gcc/testsuite/g++.old-deja/g++.other/cond5.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/cond5.C	(revision 120347)
+++ gcc/testsuite/g++.old-deja/g++.other/cond5.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do assemble  }
-// { dg-options "-W -pedantic -ansi" }
+// { dg-options "-Wextra -pedantic -ansi" }
 
 // Copyright (C) 1999, 2000 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 1 Sep 1999 <nathan@acm.org>
@@ -13,8 +13,6 @@
 // got lost due to changes in add_builtin_candidate and subsequent changes.
 
 struct X {};
-enum E1 {e1 = -1};
-enum E2 {e2 = -1};
 void f(int, ...);
 
 void fn(int i)
@@ -22,13 +20,6 @@ void fn(int i)
   double d;
   int j;
 
-  j = (i ? e1 : e2);    // { dg-warning "" } mismatch
-  d = (i ? e1 : 1.0);   // { dg-warning "" } mismatch
-  d = (i ? 1.0 : e2);   // { dg-warning "" } mismatch
-  E1 e = (i ? e1 : e1); // ok
-  j = (i ? 1 : e2);     // ok
-  j = (i ? e1 : 1);     // ok
-  
   j = (i ? throw X() : 1); // ok, int
   j = (i ? 1 : throw X()); // ok, int
   
Index: gcc/testsuite/g++.dg/warn/Wenum-promotion-Wextra.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wenum-promotion-Wextra.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wenum-promotion-Wextra.C	(revision 0)
@@ -0,0 +1,26 @@
+// Test -Wenum-promotion is enabled by -Wextra
+// { dg-do compile }
+// { dg-do "-Wextra" }
+
+enum E1 {e1 = -1};
+enum E2 {e2 = -1};
+void fn(int i, double r)
+{
+  double d;
+  int j;
+
+  j = (i ? e1 : e2);    // { dg-warning "enumeral mismatch in conditional expression: 'E1' vs 'E2'" } unconditional
+  j = (i ? e1 : e1);    
+  E1 e = (i ? e1 : e1); 
+
+  j = (i ? 1 : e2);     // ok
+  j = (i ? e1 : 1);     // ok
+  j = (i ? i : e2);     // ok
+  j = (i ? e1 : i);     // ok
+
+  d = (i ? e1 : 1.0);// { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? 1.0 : e2);// { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? e1 : r);  // { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? r : e2);  // { dg-warning "enumeral and non-enumeral type in conditional expression" }
+
+}
Index: gcc/testsuite/g++.dg/warn/Wenum-promotion-no.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wenum-promotion-no.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wenum-promotion-no.C	(revision 0)
@@ -0,0 +1,26 @@
+// Test disabling -Wenum-promotion
+// { dg-do compile }
+// { dg-do "-Wextra -Wno-enum-promotion" }
+
+enum E1 {e1 = -1};
+enum E2 {e2 = -1};
+void fn(int i, double r)
+{
+  double d;
+  int j;
+
+  j = (i ? e1 : e2);    // { dg-warning "enumeral mismatch in conditional expression: 'E1' vs 'E2'" } unconditional
+  j = (i ? e1 : e1);    
+  E1 e = (i ? e1 : e1); 
+
+  j = (i ? 1 : e2);     // ok
+  j = (i ? e1 : 1);     // ok
+  j = (i ? i : e2);     // ok
+  j = (i ? e1 : i);     // ok
+
+  d = (i ? e1 : 1.0);// { dg-bogus "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? 1.0 : e2);// { dg-bogus "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? e1 : r);  // { dg-bogus "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? r : e2);  // { dg-bogus "enumeral and non-enumeral type in conditional expression" }
+
+}
Index: gcc/testsuite/g++.dg/warn/Wenum-promotion.C
===================================================================
--- gcc/testsuite/g++.dg/warn/Wenum-promotion.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/Wenum-promotion.C	(revision 0)
@@ -0,0 +1,25 @@
+// { dg-do compile }
+// { dg-do "-Wenum-promotion" }
+
+enum E1 {e1 = -1};
+enum E2 {e2 = -1};
+void fn(int i, double r)
+{
+  double d;
+  int j;
+
+  j = (i ? e1 : e2);    // { dg-warning "enumeral mismatch in conditional expression: 'E1' vs 'E2'" } unconditional
+  j = (i ? e1 : e1);    
+  E1 e = (i ? e1 : e1); 
+
+  j = (i ? 1 : e2);     // ok
+  j = (i ? e1 : 1);     // ok
+  j = (i ? i : e2);     // ok
+  j = (i ? e1 : i);     // ok
+
+  d = (i ? e1 : 1.0);// { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? 1.0 : e2);// { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? e1 : r);  // { dg-warning "enumeral and non-enumeral type in conditional expression" } 
+  d = (i ? r : e2);  // { dg-warning "enumeral and non-enumeral type in conditional expression" }
+
+}
Index: gcc/cp/call.c
===================================================================
--- gcc/cp/call.c	(revision 120347)
+++ gcc/cp/call.c	(working copy)
@@ -3495,12 +3495,12 @@ build_conditional_expr (tree arg1, tree 
 	  && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
 	 warning (0, "enumeral mismatch in conditional expression: %qT vs %qT",
 		   arg2_type, arg3_type);
-      else if (extra_warnings
+      else if (warn_enum_promotion
 	       && ((TREE_CODE (arg2_type) == ENUMERAL_TYPE
 		    && !same_type_p (arg3_type, type_promotes_to (arg2_type)))
 		   || (TREE_CODE (arg3_type) == ENUMERAL_TYPE
 		       && !same_type_p (arg2_type, type_promotes_to (arg3_type)))))
-	warning (0, "enumeral and non-enumeral type in conditional expression");
+	warning (OPT_Wenum_promotion, "enumeral and non-enumeral type in conditional expression");
 
       arg2 = perform_implicit_conversion (result_type, arg2);
       arg3 = perform_implicit_conversion (result_type, arg3);
Index: gcc/c.opt
===================================================================
--- gcc/c.opt	(revision 120347)
+++ gcc/c.opt	(working copy)
@@ -185,6 +185,10 @@ Wendif-labels
 C ObjC C++ ObjC++
 Warn about stray tokens after #elif and #endif
 
+Wenum-promotion
+C++ ObjC++ Var(warn_enum_promotion) Init(-1)
+Warn if an enum type and a non-enum type both appear in a conditional expression
+
 Werror
 C ObjC C++ ObjC++
 ; Documented in common.opt
Index: gcc/c-opts.c
===================================================================
--- gcc/c-opts.c	(revision 120347)
+++ gcc/c-opts.c	(working copy)
@@ -1025,7 +1025,8 @@ c_common_post_options (const char **pfil
   if (flag_objc_exceptions && !flag_objc_sjlj_exceptions)
     flag_exceptions = 1;
 
-  /* -Wextra implies -Wclobbered, -Wempty-body, -Wsign-compare, 
+  /* -Wextra implies -Wclobbered, -Wempty-body, 
+     -Wenum-promotion, -Wsign-compare, 
      -Wmissing-field-initializers, -Wmissing-parameter-type
      -Wold-style-declaration, and -Woverride-init, 
      but not if explicitly overridden.  */
@@ -1033,6 +1034,8 @@ c_common_post_options (const char **pfil
     warn_clobbered = extra_warnings;
   if (warn_empty_body == -1)
     warn_empty_body = extra_warnings;
+  if (warn_enum_promotion == -1)
+    warn_enum_promotion = extra_warnings;
   if (warn_sign_compare == -1)
     warn_sign_compare = extra_warnings;
   if (warn_missing_field_initializers == -1)

Re: Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

[Gabriel Dos Reis]

"Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:

| :ADDPATCH c++:
| 
| This patch continues the effort to fix PR7651 [1].
| 
| A new option -Wenum-promotion takes over the warning for an enumerator
| and a non-enumerator appearing in a conditional expression.

I would like to see this case instead covered by the conversion/coercion
warning switch.  

Yes, I know you're trying to preserve existing practice; but we have
to keep in the fore that the current situation is not the result of
systematic principles, and now is a good to time to rationalize all
that (and thanks for your contribution in this area!).

-- Gaby

Re: Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

[Manuel López-Ibáñez]

On 07 Jan 2007 14:40:55 +0100, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
> "Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:
>
> | :ADDPATCH c++:
> |
> | This patch continues the effort to fix PR7651 [1].
> |
> | A new option -Wenum-promotion takes over the warning for an enumerator
> | and a non-enumerator appearing in a conditional expression.
>
> I would like to see this case instead covered by the conversion/coercion
> warning switch.
>
> Yes, I know you're trying to preserve existing practice; but we have
> to keep in the fore that the current situation is not the result of
> systematic principles, and now is a good to time to rationalize all
> that (and thanks for your contribution in this area!).

That could be done but such patch depends on
http://gcc.gnu.org/ml/gcc-patches/2006-12/msg00799.html being approved
first.

Also, would this warning make sense for the C front-end ?

Cheers,

Manuel.

Re: Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

[Manuel López-Ibáñez]

Also, it would be great if you could give me an update on PR12242. Do
you have a half-backed patch? What do you want to implement exactly?
Would some of it apply to the C front-end?

Cheers,

Manuel.

On 07/01/07, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 07 Jan 2007 14:40:55 +0100, Gabriel Dos Reis
> <gdr@integrable-solutions.net> wrote:
> > "Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:
> >
> > | :ADDPATCH c++:
> > |
> > | This patch continues the effort to fix PR7651 [1].
> > |
> > | A new option -Wenum-promotion takes over the warning for an enumerator
> > | and a non-enumerator appearing in a conditional expression.
> >
> > I would like to see this case instead covered by the conversion/coercion
> > warning switch.
> >
> > Yes, I know you're trying to preserve existing practice; but we have
> > to keep in the fore that the current situation is not the result of
> > systematic principles, and now is a good to time to rationalize all
> > that (and thanks for your contribution in this area!).
>
> That could be done but such patch depends on
> http://gcc.gnu.org/ml/gcc-patches/2006-12/msg00799.html being approved
> first.
>
> Also, would this warning make sense for the C front-end ?
>
> Cheers,
>
> Manuel.
>

Re: Wenum-promotion (PR7651 Define -Wextra strictly in terms of other warning flags)

[Gabriel Dos Reis]

"Manuel López-Ibáñez" <lopezibanez@gmail.com> writes:

| Also, it would be great if you could give me an update on PR12242. Do
| you have a half-backed patch? What do you want to implement exactly?

I think I lost the tree that contained the patch when moving to new
computer.  I must have it somewhere in the backups.

| Would some of it apply to the C front-end?

I think the C compiler beahviour is different, so I'm not sure it
would make much sense there.

-- Gaby