diff mbox

[C++] PR 53524

Message ID 4FC74B63.3020005@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 31, 2012, 10:43 a.m. UTC 
Hi,

when I fixed PR16603 (for 4.7.0) I didn't anticipate that we would warn 
more easily about mismatching enum types for user code using conditional 
expressions to define enumerators basing on other enumerators of the 
same open enum, like the testcase in this PR shows. Generally speaking, 
IMHO the warning is a bit pedantic, eg, the EDG front-end doesn't warn 
at all, thus it seems to me that a good thing to do, safe for 4.7.1 too, 
is giving the warning a unique name and moving it to -Wall.

Bootstrapped and tested x86_64-linux.

Thanks,
Paolo.

////////////////////////////
2012-05-31  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* doc/invoke.texi (Wenum-mismatch): Document.

/cp
2012-05-31  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* call.c (build_conditional_expr_1): Use OPT_Wenum_mismatch.

/c-family
2012-05-31  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* c.opt (Wenum-mismatch): Add.

/testsuite
2012-05-31  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* g++.dg/warn/Wenum-mismatch1.C: New.
	* g++.dg/warn/Wenum-mismatch2.C: Likewise.
	* g++.dg/warn/Wenum-mismatch3.C: Likewise.
	* g++.old-deja/g++.other/cond5.C: Adjust.

Comments

Jason Merrill May 31, 2012, 2:43 p.m. UTC | #1 
Does the C front end warn about this mismatch?  Do we warn about 
mismatch in comparisons?

Jason
Paolo Carlini May 31, 2012, 3 p.m. UTC | #2 
(resending to the mailing list because due to html content)

On 05/31/2012 04:43 PM, Jason Merrill wrote:
> Does the C front end warn about this mismatch?

I just tried the first test of g++.old-deja/g++.other/cond5.C and the C 
front-end does *not* warn neither by default, neither with -Wall.

>   Do we warn about mismatch in comparisons?

Comparisons are dealt with separately, we have -Wenum-compare:

|-Wenum-compare|
    Warn about a comparison between values of different enumerated
    types. In C++ this warning is enabled by default. In C this warning
    is enabled by -Wall.

Paolo.
Jason Merrill June 4, 2012, 4:55 p.m. UTC | #3 
On 05/31/2012 10:55 AM, Paolo Carlini wrote:
> On 05/31/2012 04:43 PM, Jason Merrill wrote:
>> Does the C front end warn about this mismatch?
> I just tried the first test of g++.old-deja/g++.other/cond5.C and the C
> front-end does *not* warn neither by default, neither with -Wall.
>>   Do we warn about mismatch in comparisons?
> Comparisons are dealt with separately, we have -Wenum-compare:

It seems to me that these should be covered by the same flag.  Maybe 
just add the warning to -Wenum-compare to start with; this isn't exactly 
comparison, but it's another case of the usual arithmetic conversions 
bringing them to a common type.  For 4.8 I think we also want to improve 
our handling of use of enums within their enum-specifier.

Jason
Paolo Carlini June 4, 2012, 5:52 p.m. UTC | #4 
Hi,

On 06/04/2012 06:55 PM, Jason Merrill wrote:
> On 05/31/2012 10:55 AM, Paolo Carlini wrote:
>> On 05/31/2012 04:43 PM, Jason Merrill wrote:
>>> Does the C front end warn about this mismatch?
>> I just tried the first test of g++.old-deja/g++.other/cond5.C and the C
>> front-end does *not* warn neither by default, neither with -Wall.
>>>   Do we warn about mismatch in comparisons?
>> Comparisons are dealt with separately, we have -Wenum-compare:
>
> It seems to me that these should be covered by the same flag.  Maybe 
> just add the warning to -Wenum-compare to start with; this isn't 
> exactly comparison, but it's another case of the usual arithmetic 
> conversions bringing them to a common type.
Ok, this would be simple to do. The only issue I can see, is that in C++ 
-Wenum-compare has a name, thus can be easily disabled but it's ON by 
default. Anyway, I'll send a patchlet should be just a couple of lines 
in call.c + a few words in the doc.

Paolo.
Jason Merrill June 4, 2012, 6:24 p.m. UTC | #5 
On 06/04/2012 01:52 PM, Paolo Carlini wrote:
> Ok, this would be simple to do. The only issue I can see, is that in C++
> -Wenum-compare has a name, thus can be easily disabled but it's ON by
> default.

The warning is already on by default, so that wouldn't be a change; this 
just creates a way for users to turn it off until we deal with the 
unhelpful case.

Jason
diff mbox

Patch

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 188050)
+++ doc/invoke.texi	(working copy)
@@ -198,8 +198,8 @@  in the following sections.
 -fno-default-inline  -fvisibility-inlines-hidden @gol
 -fvisibility-ms-compat @gol
 -Wabi  -Wconversion-null  -Wctor-dtor-privacy @gol
--Wdelete-non-virtual-dtor -Wliteral-suffix -Wnarrowing @gol
--Wnoexcept -Wnon-virtual-dtor  -Wreorder @gol
+-Wdelete-non-virtual-dtor  -Wenum-mismatch  -Wliteral-suffix @gol
+-Wnarrowing  -Wnoexcept  -Wnon-virtual-dtor  -Wreorder @gol
 -Weffc++  -Wstrict-null-sentinel @gol
 -Wno-non-template-friend  -Wold-style-cast @gol
 -Woverloaded-virtual  -Wno-pmf-conversions @gol
@@ -3084,6 +3084,7 @@  Options} and @ref{Objective-C and Objective-C++ Di
 -Wc++11-compat  @gol
 -Wchar-subscripts  @gol
 -Wenum-compare @r{(in C/Objc; this is on by default in C++)} @gol
+-Wenum-mismatch @r{(in C++)} @gol
 -Wimplicit-int @r{(C and Objective-C only)} @gol
 -Wimplicit-function-declaration @r{(C and Objective-C only)} @gol
 -Wcomment  @gol
@@ -4301,6 +4302,12 @@  Warn about a comparison between values of differen
 this warning is enabled by default.  In C this warning is enabled by
 @option{-Wall}.
 
+@item -Wenum-mismatch @r{(C++ and Objective-C++ only)}
+@opindex Wenum-mismatch
+@opindex Wno-enum-mismatch
+Warn about values of different enumerated types in conditional
+expressions.  This warning is enabled by @option{-Wall}.
+
 @item -Wjump-misses-init @r{(C, Objective-C only)}
 @opindex Wjump-misses-init
 @opindex Wno-jump-misses-init
Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 188053)
+++ c-family/c.opt	(working copy)
@@ -360,6 +360,10 @@  Wenum-compare
 C ObjC C++ ObjC++ Var(warn_enum_compare) Init(-1) Warning
 Warn about comparison of different enum types
 
+Wenum-mismatch
+C++ ObjC++ Var(warn_enum_mismatch) Warning LangEnabledBy(C++ ObjC++,Wall)
+Warn about enumeral mismatch in conditional expression
+
 Werror
 C ObjC C++ ObjC++
 ; Documented in common.opt
Index: testsuite/g++.old-deja/g++.other/cond5.C
===================================================================
--- testsuite/g++.old-deja/g++.other/cond5.C	(revision 188050)
+++ testsuite/g++.old-deja/g++.other/cond5.C	(working copy)
@@ -1,7 +1,7 @@ 
 // { dg-do assemble  }
-// { dg-options "-W -pedantic -ansi" }
+// { dg-options "-W -Wenum-mismatch -pedantic -ansi" }
 
-// Copyright (C) 1999, 2000 Free Software Foundation, Inc.
+// Copyright (C) 1999, 2000, 2012 Free Software Foundation, Inc.
 // Contributed by Nathan Sidwell 1 Sep 1999 <nathan@acm.org>
 // Derived from bug report from Gabriel Dos Reis
 // <Gabriel.Dos-Reis@cmla.ens-cachan.fr>
Index: testsuite/g++.dg/warn/Wenum-mismatch1.C
===================================================================
--- testsuite/g++.dg/warn/Wenum-mismatch1.C	(revision 0)
+++ testsuite/g++.dg/warn/Wenum-mismatch1.C	(revision 0)
@@ -0,0 +1,30 @@ 
+// PR c++/53524
+
+template < typename > struct PointerLikeTypeTraits {
+  enum { NumLowBitsAvailable };
+};
+
+class CodeGenInstruction;
+class CodeGenInstAlias;
+
+template < typename T>
+struct PointerIntPair {
+  enum { IntShift = T::NumLowBitsAvailable };
+};
+
+template < typename PT1, typename PT2 > struct PointerUnionUIntTraits {
+  enum {
+    PT1BitsAv = PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable,
+    PT2BitsAv = PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable,
+    NumLowBitsAvailable = 0 ? PT1BitsAv : PT2BitsAv
+  };
+};
+
+template < typename PT1, typename PT2 > class PointerUnion {
+  typedef PointerIntPair < PointerUnionUIntTraits < PT1, PT2 > > ValTy;
+  ValTy Val;
+};
+
+struct ClassInfo {
+  PointerUnion < CodeGenInstruction *, CodeGenInstAlias * > DefRec;
+};
Index: testsuite/g++.dg/warn/Wenum-mismatch2.C
===================================================================
--- testsuite/g++.dg/warn/Wenum-mismatch2.C	(revision 0)
+++ testsuite/g++.dg/warn/Wenum-mismatch2.C	(revision 0)
@@ -0,0 +1,31 @@ 
+// PR c++/53524
+// { dg-options "-Wenum-mismatch" }
+
+template < typename > struct PointerLikeTypeTraits {
+  enum { NumLowBitsAvailable };
+};
+
+class CodeGenInstruction;
+class CodeGenInstAlias;
+
+template < typename T>
+struct PointerIntPair {
+  enum { IntShift = T::NumLowBitsAvailable };
+};
+
+template < typename PT1, typename PT2 > struct PointerUnionUIntTraits {
+  enum {
+    PT1BitsAv = PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable,
+    PT2BitsAv = PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable,
+    NumLowBitsAvailable = 0 ? PT1BitsAv : PT2BitsAv  // { dg-warning "enumeral mismatch" }
+  };
+};
+
+template < typename PT1, typename PT2 > class PointerUnion {
+  typedef PointerIntPair < PointerUnionUIntTraits < PT1, PT2 > > ValTy;
+  ValTy Val;
+};
+
+struct ClassInfo {
+  PointerUnion < CodeGenInstruction *, CodeGenInstAlias * > DefRec;
+};
Index: testsuite/g++.dg/warn/Wenum-mismatch3.C
===================================================================
--- testsuite/g++.dg/warn/Wenum-mismatch3.C	(revision 0)
+++ testsuite/g++.dg/warn/Wenum-mismatch3.C	(revision 0)
@@ -0,0 +1,31 @@ 
+// PR c++/53524
+// { dg-options "-Wall" }
+
+template < typename > struct PointerLikeTypeTraits {
+  enum { NumLowBitsAvailable };
+};
+
+class CodeGenInstruction;
+class CodeGenInstAlias;
+
+template < typename T>
+struct PointerIntPair {
+  enum { IntShift = T::NumLowBitsAvailable };
+};
+
+template < typename PT1, typename PT2 > struct PointerUnionUIntTraits {
+  enum {
+    PT1BitsAv = PointerLikeTypeTraits < PT1 >::NumLowBitsAvailable,
+    PT2BitsAv = PointerLikeTypeTraits < PT2 >::NumLowBitsAvailable,
+    NumLowBitsAvailable = 0 ? PT1BitsAv : PT2BitsAv  // { dg-warning "enumeral mismatch" }
+  };
+};
+
+template < typename PT1, typename PT2 > class PointerUnion {
+  typedef PointerIntPair < PointerUnionUIntTraits < PT1, PT2 > > ValTy;
+  ValTy Val;
+};
+
+struct ClassInfo {
+  PointerUnion < CodeGenInstruction *, CodeGenInstAlias * > DefRec;
+};
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 188053)
+++ cp/call.c	(working copy)
@@ -4697,7 +4697,7 @@  build_conditional_expr_1 (tree arg1, tree arg2, tr
 	  && TREE_CODE (arg3_type) == ENUMERAL_TYPE)
         {
           if (complain & tf_warning)
-            warning (0, 
+            warning (OPT_Wenum_mismatch, 
                      "enumeral mismatch in conditional expression: %qT vs %qT",
                      arg2_type, arg3_type);
         }