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

[Jonathan Wakely]

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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.txt
Type: text/x-patch
Size: 3575 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20151112/84a46456/attachment.bin>