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

Jonathan Wakely jwakely@redhat.com
Wed Nov 25 13:54:00 GMT 2015


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



More information about the Gcc-patches mailing list