[patch] libstdc++/56158 Extend valid values of iostream bitmask types

[patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Jonathan Wakely]

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

As described in the PR, we have operator~ overloads defined for
enumeration types which produce values outside the range of valid
values for the type. In C++11 that can be trivially solved by giving
the enumeration types a fixed underlying type, but this code needs to
be valid in C++03 too.

This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
every int value is also a valid value for the bitmask type.

Does anyone see any problems with this solution, or better solutions?

Any suggestions for how to test this, given that GCC's ubsan doesn't
check for this, and we can't run the testsuite with ubsan anyway?


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 3590 bytes --]

commit ac2fc08a638c7f01ecb5b9e9b3d3a58caf031534
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Nov 11 09:25:38 2015 +0000

    Extend valid values of iostream bitmask types
    
    	PR libstdc++/56158
    	* include/bits/ios_base.h (_Ios_Fmtflags, _Ios_Openmode, _Ios_Iostate):
    	Define enumerators to ensure all values of type int are valid values
    	of the enumeration type.
    	* testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Add new cases.
    	* testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise.
    	* testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-v3/include/bits/ios_base.h
index 44029ad..ba4ef92 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -74,7 +74,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_adjustfield 	= _S_left | _S_right | _S_internal,
       _S_basefield 	= _S_dec | _S_oct | _S_hex,
       _S_floatfield 	= _S_scientific | _S_fixed,
-      _S_ios_fmtflags_end = 1L << 16 
+      _S_ios_fmtflags_end = 1L << 16,
+      _S_ios_fmtflags_max = __INT_MAX__,
+      _S_ios_fmtflags_min = ~(int)__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Fmtflags
@@ -114,7 +116,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_in 		= 1L << 3,
       _S_out 		= 1L << 4,
       _S_trunc 		= 1L << 5,
-      _S_ios_openmode_end = 1L << 16 
+      _S_ios_openmode_end = 1L << 16,
+      _S_ios_openmode_max = __INT_MAX__,
+      _S_ios_openmode_min = ~(int)__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Openmode
@@ -152,7 +156,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_badbit 		= 1L << 0,
       _S_eofbit 		= 1L << 1,
       _S_failbit		= 1L << 2,
-      _S_ios_iostate_end = 1L << 16 
+      _S_ios_iostate_end = 1L << 16,
+      _S_ios_iostate_max = __INT_MAX__,
+      _S_ios_iostate_min = ~(int)__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Iostate
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
index 591e371..e8820c5 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
@@ -70,5 +70,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_fmtflags_end:
       break;
+    case std::_S_ios_fmtflags_min:
+      break;
+    case std::_S_ios_fmtflags_max:
+      break;
     }
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
index 44fb44e..4e4e4f5 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
@@ -42,5 +42,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_iostate_end:
       break;
+    case std::_S_ios_iostate_min:
+      break;
+    case std::_S_ios_iostate_max:
+      break;
     }
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
index 267f8a2..8c6672f6 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
@@ -46,5 +46,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_openmode_end:
       break;
+    case std::_S_ios_openmode_min:
+      break;
+    case std::_S_ios_openmode_max:
+      break;
     }
 }

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Martin Sebor]

On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
> As described in the PR, we have operator~ overloads defined for
> enumeration types which produce values outside the range of valid
> values for the type. In C++11 that can be trivially solved by giving
> the enumeration types a fixed underlying type, but this code needs to
> be valid in C++03 too.
>
> This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
> every int value is also a valid value for the bitmask type.
>
> Does anyone see any problems with this solution, or better solutions?

Just a minor nit that the C-style cast in the below triggers
a -Wold-style-cast warning in Clang, in case libstdc++ tries
to be Clang-warning free. Since the type of __INT_MAX__ is
int it shouldn't be necessary.

+      _S_ios_fmtflags_min = ~(int)__INT_MAX__

>
> Any suggestions for how to test this, given that GCC's ubsan doesn't
> check for this, and we can't run the testsuite with ubsan anyway?

Use a case/switch statement with -Werror=switch-enum to make sure
all the cases are handled and none is duplicated or outside of the
valid values of the enumeration:

   void foo (ios::iostate s) {
       switch (s) {
       case badbit:
       case eofbit:
       case failbit:
       case goodbit:
       case __INT_MAX__:
       case ~__INT_MAX__: ;
     }
   }

Martin

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Jonathan Wakely]

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

On 12/11/15 08:48 -0700, Martin Sebor wrote:
>On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
>>As described in the PR, we have operator~ overloads defined for
>>enumeration types which produce values outside the range of valid
>>values for the type. In C++11 that can be trivially solved by giving
>>the enumeration types a fixed underlying type, but this code needs to
>>be valid in C++03 too.
>>
>>This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
>>every int value is also a valid value for the bitmask type.
>>
>>Does anyone see any problems with this solution, or better solutions?
>
>Just a minor nit that the C-style cast in the below triggers
>a -Wold-style-cast warning in Clang, in case libstdc++ tries
>to be Clang-warning free. Since the type of __INT_MAX__ is
>int it shouldn't be necessary.
>
>+      _S_ios_fmtflags_min = ~(int)__INT_MAX__

That's worth fixing, thanks.

>>
>>Any suggestions for how to test this, given that GCC's ubsan doesn't
>>check for this, and we can't run the testsuite with ubsan anyway?
>
>Use a case/switch statement with -Werror=switch-enum to make sure
>all the cases are handled and none is duplicated or outside of the
>valid values of the enumeration:
>
>  void foo (ios::iostate s) {
>      switch (s) {
>      case badbit:
>      case eofbit:
>      case failbit:
>      case goodbit:
>      case __INT_MAX__:
>      case ~__INT_MAX__: ;
>    }
>  }

I thought this was a great idea at first ... but -Wswitch-enum will
complain that the end, min and max enumerators are not handled (even
though __INT_MAX__ and ~__INT_MAX__ have the same values as the max
and min ones, respectively).

We could use something like this:

  switch(f()) {
  case ~ios::iostate():
  default:
    break;
  }

Which with no warning options at all (or with -Wno-switch to suppress
other warnings) gives this for the buggy code currently on trunk:

e.cc:13:2: warning: case label value is less than minimum value for type
  case ~ios::iostate():
  ^

But that could break if someone fixes that warning to depend on
-Wswitch (which it probably should do). And with -Wswitch that gives a
different warning:

e.cc:10:3: warning: case value ‘-1’ not in enumerated type ‘E’ [-Wswitch]

So I think I'll just leave the tests as they are, and rely on our
diligent Clang users to report if there's still a problem :-)

I've committed the attached patch to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 3575 bytes --]

commit 2ec414f95401368dc532b3fc6155ed7be41edb8e
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 12 16:16:20 2015 +0000

    Extend valid values of iostream bitmask types
    
    	PR libstdc++/56158
    	* include/bits/ios_base.h (_Ios_Fmtflags, _Ios_Openmode, _Ios_Iostate):
    	Define enumerators to ensure all values of type int are valid values
    	of the enumeration type.
    	* testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Add new cases.
    	* testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise.
    	* testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise.

diff --git a/libstdc++-v3/include/bits/ios_base.h b/libstdc++-v3/include/bits/ios_base.h
index 44029ad..908ba7c 100644
--- a/libstdc++-v3/include/bits/ios_base.h
+++ b/libstdc++-v3/include/bits/ios_base.h
@@ -74,7 +74,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_adjustfield 	= _S_left | _S_right | _S_internal,
       _S_basefield 	= _S_dec | _S_oct | _S_hex,
       _S_floatfield 	= _S_scientific | _S_fixed,
-      _S_ios_fmtflags_end = 1L << 16 
+      _S_ios_fmtflags_end = 1L << 16,
+      _S_ios_fmtflags_max = __INT_MAX__,
+      _S_ios_fmtflags_min = ~__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Fmtflags
@@ -114,7 +116,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_in 		= 1L << 3,
       _S_out 		= 1L << 4,
       _S_trunc 		= 1L << 5,
-      _S_ios_openmode_end = 1L << 16 
+      _S_ios_openmode_end = 1L << 16,
+      _S_ios_openmode_max = __INT_MAX__,
+      _S_ios_openmode_min = ~__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Openmode
@@ -152,7 +156,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       _S_badbit 		= 1L << 0,
       _S_eofbit 		= 1L << 1,
       _S_failbit		= 1L << 2,
-      _S_ios_iostate_end = 1L << 16 
+      _S_ios_iostate_end = 1L << 16,
+      _S_ios_iostate_max = __INT_MAX__,
+      _S_ios_iostate_min = ~__INT_MAX__
     };
 
   inline _GLIBCXX_CONSTEXPR _Ios_Iostate
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
index 591e371..e8820c5 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
@@ -70,5 +70,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_fmtflags_end:
       break;
+    case std::_S_ios_fmtflags_min:
+      break;
+    case std::_S_ios_fmtflags_max:
+      break;
     }
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
index 44fb44e..4e4e4f5 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
@@ -42,5 +42,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_iostate_end:
       break;
+    case std::_S_ios_iostate_min:
+      break;
+    case std::_S_ios_iostate_max:
+      break;
     }
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
index 267f8a2..8c6672f6 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
@@ -46,5 +46,9 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_openmode_end:
       break;
+    case std::_S_ios_openmode_min:
+      break;
+    case std::_S_ios_openmode_max:
+      break;
     }
 }

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Martin Sebor]

On 11/12/2015 10:08 AM, Jonathan Wakely wrote:
> On 12/11/15 08:48 -0700, Martin Sebor wrote:
>> On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
>>> As described in the PR, we have operator~ overloads defined for
>>> enumeration types which produce values outside the range of valid
>>> values for the type. In C++11 that can be trivially solved by giving
>>> the enumeration types a fixed underlying type, but this code needs to
>>> be valid in C++03 too.
>>>
>>> This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
>>> every int value is also a valid value for the bitmask type.
>>>
>>> Does anyone see any problems with this solution, or better solutions?
>>
>> Just a minor nit that the C-style cast in the below triggers
>> a -Wold-style-cast warning in Clang, in case libstdc++ tries
>> to be Clang-warning free. Since the type of __INT_MAX__ is
>> int it shouldn't be necessary.
>>
>> +      _S_ios_fmtflags_min = ~(int)__INT_MAX__
>
> That's worth fixing, thanks.
>
>>>
>>> Any suggestions for how to test this, given that GCC's ubsan doesn't
>>> check for this, and we can't run the testsuite with ubsan anyway?
>>
>> Use a case/switch statement with -Werror=switch-enum to make sure
>> all the cases are handled and none is duplicated or outside of the
>> valid values of the enumeration:
>>
>>  void foo (ios::iostate s) {
>>      switch (s) {
>>      case badbit:
>>      case eofbit:
>>      case failbit:
>>      case goodbit:
>>      case __INT_MAX__:
>>      case ~__INT_MAX__: ;
>>    }
>>  }
>
> I thought this was a great idea at first ... but -Wswitch-enum will
> complain that the end, min and max enumerators are not handled (even
> though __INT_MAX__ and ~__INT_MAX__ have the same values as the max
> and min ones, respectively).

Hmm, I didn't see any warnings for the small test case I wrote and
still don't. Just out of curiosity, what did I miss?

enum iostate {
     goodbit = 0,
     eofbit,
     failbit,
     badbit,
     max = __INT_MAX__,
     min = ~__INT_MAX__
};

void foo (iostate s) {
     switch (s) {
     case badbit:
     case eofbit:
     case failbit:
     case goodbit:
     case __INT_MAX__:
     case ~__INT_MAX__: ;
     ;
     }
}

Martin

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Jonathan Wakely]

On 12/11/15 11:09 -0700, Martin Sebor wrote:
>On 11/12/2015 10:08 AM, Jonathan Wakely wrote:
>>On 12/11/15 08:48 -0700, Martin Sebor wrote:
>>>On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
>>>>As described in the PR, we have operator~ overloads defined for
>>>>enumeration types which produce values outside the range of valid
>>>>values for the type. In C++11 that can be trivially solved by giving
>>>>the enumeration types a fixed underlying type, but this code needs to
>>>>be valid in C++03 too.
>>>>
>>>>This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
>>>>every int value is also a valid value for the bitmask type.
>>>>
>>>>Does anyone see any problems with this solution, or better solutions?
>>>
>>>Just a minor nit that the C-style cast in the below triggers
>>>a -Wold-style-cast warning in Clang, in case libstdc++ tries
>>>to be Clang-warning free. Since the type of __INT_MAX__ is
>>>int it shouldn't be necessary.
>>>
>>>+      _S_ios_fmtflags_min = ~(int)__INT_MAX__
>>
>>That's worth fixing, thanks.
>>
>>>>
>>>>Any suggestions for how to test this, given that GCC's ubsan doesn't
>>>>check for this, and we can't run the testsuite with ubsan anyway?
>>>
>>>Use a case/switch statement with -Werror=switch-enum to make sure
>>>all the cases are handled and none is duplicated or outside of the
>>>valid values of the enumeration:
>>>
>>> void foo (ios::iostate s) {
>>>     switch (s) {
>>>     case badbit:
>>>     case eofbit:
>>>     case failbit:
>>>     case goodbit:
>>>     case __INT_MAX__:
>>>     case ~__INT_MAX__: ;
>>>   }
>>> }
>>
>>I thought this was a great idea at first ... but -Wswitch-enum will
>>complain that the end, min and max enumerators are not handled (even
>>though __INT_MAX__ and ~__INT_MAX__ have the same values as the max
>>and min ones, respectively).
>
>Hmm, I didn't see any warnings for the small test case I wrote and
>still don't. Just out of curiosity, what did I miss?
>
>enum iostate {
>    goodbit = 0,
>    eofbit,
>    failbit,
>    badbit,
>    max = __INT_MAX__,
>    min = ~__INT_MAX__
>};
>
>void foo (iostate s) {
>    switch (s) {
>    case badbit:
>    case eofbit:
>    case failbit:
>    case goodbit:
>    case __INT_MAX__:
>    case ~__INT_MAX__: ;
>    ;
>    }
>}

I must have messed up my test, I agree this is warning clean.

So I could add tests for those minimum and maximum values, and also
static_assert that the enumerations have an underlying type with the
same representation as int (because if it's actually bigger than int
the operator~ could still produce values outside the range of valid
values).

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Jonathan Wakely]

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

On 13/11/15 08:39 +0000, Jonathan Wakely wrote:
>On 12/11/15 11:09 -0700, Martin Sebor wrote:
>>On 11/12/2015 10:08 AM, Jonathan Wakely wrote:
>>>On 12/11/15 08:48 -0700, Martin Sebor wrote:
>>>>On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
>>>>>As described in the PR, we have operator~ overloads defined for
>>>>>enumeration types which produce values outside the range of valid
>>>>>values for the type. In C++11 that can be trivially solved by giving
>>>>>the enumeration types a fixed underlying type, but this code needs to
>>>>>be valid in C++03 too.
>>>>>
>>>>>This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
>>>>>every int value is also a valid value for the bitmask type.
>>>>>
>>>>>Does anyone see any problems with this solution, or better solutions?
>>>>
>>>>Just a minor nit that the C-style cast in the below triggers
>>>>a -Wold-style-cast warning in Clang, in case libstdc++ tries
>>>>to be Clang-warning free. Since the type of __INT_MAX__ is
>>>>int it shouldn't be necessary.
>>>>
>>>>+      _S_ios_fmtflags_min = ~(int)__INT_MAX__
>>>
>>>That's worth fixing, thanks.
>>>
>>>>>
>>>>>Any suggestions for how to test this, given that GCC's ubsan doesn't
>>>>>check for this, and we can't run the testsuite with ubsan anyway?
>>>>
>>>>Use a case/switch statement with -Werror=switch-enum to make sure
>>>>all the cases are handled and none is duplicated or outside of the
>>>>valid values of the enumeration:
>>>>
>>>>void foo (ios::iostate s) {
>>>>    switch (s) {
>>>>    case badbit:
>>>>    case eofbit:
>>>>    case failbit:
>>>>    case goodbit:
>>>>    case __INT_MAX__:
>>>>    case ~__INT_MAX__: ;
>>>>  }
>>>>}
>>>
>>>I thought this was a great idea at first ... but -Wswitch-enum will
>>>complain that the end, min and max enumerators are not handled (even
>>>though __INT_MAX__ and ~__INT_MAX__ have the same values as the max
>>>and min ones, respectively).
>>
>>Hmm, I didn't see any warnings for the small test case I wrote and
>>still don't. Just out of curiosity, what did I miss?
>>
>>enum iostate {
>>   goodbit = 0,
>>   eofbit,
>>   failbit,
>>   badbit,
>>   max = __INT_MAX__,
>>   min = ~__INT_MAX__
>>};
>>
>>void foo (iostate s) {
>>   switch (s) {
>>   case badbit:
>>   case eofbit:
>>   case failbit:
>>   case goodbit:
>>   case __INT_MAX__:
>>   case ~__INT_MAX__: ;
>>   ;
>>   }
>>}
>
>I must have messed up my test, I agree this is warning clean.
>
>So I could add tests for those minimum and maximum values, and also
>static_assert that the enumerations have an underlying type with the
>same representation as int (because if it's actually bigger than int
>the operator~ could still produce values outside the range of valid
>values).

How's this?




[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2671 bytes --]

commit c9fc6b83d0ec4bbf73d7d0f410e0b0e2799d8adf
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Nov 13 08:56:22 2015 +0000

    Improve tests for valid values of iostream bitmask types
    
    	* testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Explicitly
    	check minimum and maximum values, and size of underlying type.
    	* testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise.
    	* testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise.

diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
index e8820c5..3475fd3 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
@@ -70,9 +70,11 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_fmtflags_end:
       break;
-    case std::_S_ios_fmtflags_min:
+    case __INT_MAX__:
       break;
-    case std::_S_ios_fmtflags_max:
+    case ~__INT_MAX__:
       break;
     }
+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
+      "underlying type has same range of values as int");
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
index 4e4e4f5..a72a774 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
@@ -42,9 +42,11 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_iostate_end:
       break;
-    case std::_S_ios_iostate_min:
+    case __INT_MAX__:
       break;
-    case std::_S_ios_iostate_max:
+    case ~__INT_MAX__:
       break;
     }
+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
+      "underlying type has same range of values as int");
 }
diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
index 8c6672f6..f621d21 100644
--- a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
@@ -46,9 +46,11 @@ case_labels(bitmask_type b)
       break;
     case std::_S_ios_openmode_end:
       break;
-    case std::_S_ios_openmode_min:
+    case __INT_MAX__:
       break;
-    case std::_S_ios_openmode_max:
+    case ~__INT_MAX__:
       break;
     }
+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
+      "underlying type has same range of values as int");
 }

Re: [patch] libstdc++/56158 Extend valid values of iostream bitmask types

[Jonathan Wakely]

On 13/11/15 09:11 +0000, Jonathan Wakely wrote:
>On 13/11/15 08:39 +0000, Jonathan Wakely wrote:
>>On 12/11/15 11:09 -0700, Martin Sebor wrote:
>>>On 11/12/2015 10:08 AM, Jonathan Wakely wrote:
>>>>On 12/11/15 08:48 -0700, Martin Sebor wrote:
>>>>>On 11/11/2015 02:48 AM, Jonathan Wakely wrote:
>>>>>>As described in the PR, we have operator~ overloads defined for
>>>>>>enumeration types which produce values outside the range of valid
>>>>>>values for the type. In C++11 that can be trivially solved by giving
>>>>>>the enumeration types a fixed underlying type, but this code needs to
>>>>>>be valid in C++03 too.
>>>>>>
>>>>>>This patch defines new min/max enumerators as INT_MIN/INT_MAX so that
>>>>>>every int value is also a valid value for the bitmask type.
>>>>>>
>>>>>>Does anyone see any problems with this solution, or better solutions?
>>>>>
>>>>>Just a minor nit that the C-style cast in the below triggers
>>>>>a -Wold-style-cast warning in Clang, in case libstdc++ tries
>>>>>to be Clang-warning free. Since the type of __INT_MAX__ is
>>>>>int it shouldn't be necessary.
>>>>>
>>>>>+      _S_ios_fmtflags_min = ~(int)__INT_MAX__
>>>>
>>>>That's worth fixing, thanks.
>>>>
>>>>>>
>>>>>>Any suggestions for how to test this, given that GCC's ubsan doesn't
>>>>>>check for this, and we can't run the testsuite with ubsan anyway?
>>>>>
>>>>>Use a case/switch statement with -Werror=switch-enum to make sure
>>>>>all the cases are handled and none is duplicated or outside of the
>>>>>valid values of the enumeration:
>>>>>
>>>>>void foo (ios::iostate s) {
>>>>>   switch (s) {
>>>>>   case badbit:
>>>>>   case eofbit:
>>>>>   case failbit:
>>>>>   case goodbit:
>>>>>   case __INT_MAX__:
>>>>>   case ~__INT_MAX__: ;
>>>>> }
>>>>>}
>>>>
>>>>I thought this was a great idea at first ... but -Wswitch-enum will
>>>>complain that the end, min and max enumerators are not handled (even
>>>>though __INT_MAX__ and ~__INT_MAX__ have the same values as the max
>>>>and min ones, respectively).
>>>
>>>Hmm, I didn't see any warnings for the small test case I wrote and
>>>still don't. Just out of curiosity, what did I miss?
>>>
>>>enum iostate {
>>>  goodbit = 0,
>>>  eofbit,
>>>  failbit,
>>>  badbit,
>>>  max = __INT_MAX__,
>>>  min = ~__INT_MAX__
>>>};
>>>
>>>void foo (iostate s) {
>>>  switch (s) {
>>>  case badbit:
>>>  case eofbit:
>>>  case failbit:
>>>  case goodbit:
>>>  case __INT_MAX__:
>>>  case ~__INT_MAX__: ;
>>>  ;
>>>  }
>>>}
>>
>>I must have messed up my test, I agree this is warning clean.
>>
>>So I could add tests for those minimum and maximum values, and also
>>static_assert that the enumerations have an underlying type with the
>>same representation as int (because if it's actually bigger than int
>>the operator~ could still produce values outside the range of valid
>>values).
>
>How's this?


I've committed this to trunk, and am backporting the pr56158 fix (with
these test improvements) to gcc-5-branch.


>commit c9fc6b83d0ec4bbf73d7d0f410e0b0e2799d8adf
>Author: Jonathan Wakely <jwakely@redhat.com>
>Date:   Fri Nov 13 08:56:22 2015 +0000
>
>    Improve tests for valid values of iostream bitmask types
>    
>    	* testsuite/27_io/ios_base/types/fmtflags/case_label.cc: Explicitly
>    	check minimum and maximum values, and size of underlying type.
>    	* testsuite/27_io/ios_base/types/iostate/case_label.cc: Likewise.
>    	* testsuite/27_io/ios_base/types/openmode/case_label.cc: Likewise.
>
>diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
>index e8820c5..3475fd3 100644
>--- a/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
>+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/fmtflags/case_label.cc
>@@ -70,9 +70,11 @@ case_labels(bitmask_type b)
>       break;
>     case std::_S_ios_fmtflags_end:
>       break;
>-    case std::_S_ios_fmtflags_min:
>+    case __INT_MAX__:
>       break;
>-    case std::_S_ios_fmtflags_max:
>+    case ~__INT_MAX__:
>       break;
>     }
>+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
>+      "underlying type has same range of values as int");
> }
>diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
>index 4e4e4f5..a72a774 100644
>--- a/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
>+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/iostate/case_label.cc
>@@ -42,9 +42,11 @@ case_labels(bitmask_type b)
>       break;
>     case std::_S_ios_iostate_end:
>       break;
>-    case std::_S_ios_iostate_min:
>+    case __INT_MAX__:
>       break;
>-    case std::_S_ios_iostate_max:
>+    case ~__INT_MAX__:
>       break;
>     }
>+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
>+      "underlying type has same range of values as int");
> }
>diff --git a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
>index 8c6672f6..f621d21 100644
>--- a/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
>+++ b/libstdc++-v3/testsuite/27_io/ios_base/types/openmode/case_label.cc
>@@ -46,9 +46,11 @@ case_labels(bitmask_type b)
>       break;
>     case std::_S_ios_openmode_end:
>       break;
>-    case std::_S_ios_openmode_min:
>+    case __INT_MAX__:
>       break;
>-    case std::_S_ios_openmode_max:
>+    case ~__INT_MAX__:
>       break;
>     }
>+  static_assert( sizeof(std::underlying_type_t<bitmask_type>) == sizeof(int),
>+      "underlying type has same range of values as int");
> }