[C++ Patch] PR 53524

[C++ Patch] PR 53524

[Paolo Carlini]

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

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.

////////////////////////////

[-- Attachment #2: CL_53524 --]
[-- Type: text/plain, Size: 615 bytes --]

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.

[-- Attachment #3: patch_53524 --]
[-- Type: text/plain, Size: 6555 bytes --]

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

Re: [C++ Patch] PR 53524

[Jason Merrill]

Does the C front end warn about this mismatch?  Do we warn about 
mismatch in comparisons?

Jason

Re: [C++ Patch] PR 53524

[Paolo Carlini]

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

Re: [C++ Patch] PR 53524

[Jason Merrill]

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

Re: [C++ Patch] PR 53524

[Paolo Carlini]

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.

Re: [C++ Patch] PR 53524

[Jason Merrill]

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

Re: [C++ Patch] PR 53524

[Paolo Carlini]

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

On 06/04/2012 08:24 PM, Jason Merrill wrote:
> 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.
Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and 
4.7.1?

Thanks,
Paolo.

/////////////////////

[-- Attachment #2: CL_53524_2 --]
[-- Type: text/plain, Size: 443 bytes --]

2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* doc/invoke.texi (Wenum-compare): Update documentation.

/cp
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* call.c (build_conditional_expr_1): Use OPT_Wenum_compare
	to control enumeral mismatch in conditional expression too.

/testsuite
2012-06-04  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/53524
	* g++.dg/warn/Wenum-compare-no-2: New.

[-- Attachment #3: patch_53524_2 --]
[-- Type: text/plain, Size: 2524 bytes --]

Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 188196)
+++ doc/invoke.texi	(working copy)
@@ -4297,9 +4297,10 @@ while} statement.  This warning is also enabled by
 @item -Wenum-compare
 @opindex Wenum-compare
 @opindex Wno-enum-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
-@option{-Wall}.
+Warn about a comparison between values of different enumerated types.
+In C++ enumeral mismatches in conditional expressions are also
+diagnosed and the warning is enabled by default.  In C this warning is 
+enabled by @option{-Wall}.
 
 @item -Wjump-misses-init @r{(C, Objective-C only)}
 @opindex Wjump-misses-init
Index: testsuite/g++.dg/warn/Wenum-compare-no-2.C
===================================================================
--- testsuite/g++.dg/warn/Wenum-compare-no-2.C	(revision 0)
+++ testsuite/g++.dg/warn/Wenum-compare-no-2.C	(revision 0)
@@ -0,0 +1,31 @@
+// PR c++/53524
+// { dg-options "-Wno-enum-compare" }
+
+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/cpp0x/alias-decl-19.C
===================================================================
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 188196)
+++ cp/call.c	(working copy)
@@ -4696,7 +4696,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_compare, 
                      "enumeral mismatch in conditional expression: %qT vs %qT",
                      arg2_type, arg3_type);
         }

Re: [C++ Patch] PR 53524

[Jason Merrill]

On 06/04/2012 02:50 PM, Paolo Carlini wrote:
> Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and
> 4.7.1?

OK.  But let's leave the bug open until we fix the warning not to 
complain about this testcase even with -Wenum-compare on.

Jason

Re: [C++ Patch] PR 53524

[Paolo Carlini]

On 06/04/2012 09:16 PM, Jason Merrill wrote:
> On 06/04/2012 02:50 PM, Paolo Carlini wrote:
>> Ok. The below passes the testsuite on x86_64-linux. Ok for mainline and
>> 4.7.1?
>
> OK.  But let's leave the bug open until we fix the warning not to 
> complain about this testcase even with -Wenum-compare on.
Sure.

Paolo.