http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52763
--- Comment #5 from Mikka 2012-04-16 11:01:25 UTC
---
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > But what about cases such as (val1 == (ONE|TWO)) ?
> > >
> > > (ONE|TWO) is of type 'int' but that code is correct and shouldn't warn
> >
> > In my opinion, there should be a warning here, because (ONE|TWO) is not in
> > the
> > enumeration range (there is no definition for 3).
>
> It is in range. For enum { NONE = 0, ONE = 1, TWO = 2 } the standard says the
> values of the enumeration are in the range 0 to 3, e.g. it could be
> represented
> by a two-bit unsigned integer. The standard sets the rules, not your opinion
> :)
>
While the standard says, it should be an integer of this range. It does not
say, that the enumeration-type should be able/used to store other values as the
ones, which are defined. Actually it does say, that there is not conversion
from int to enum. So this means here, that we would compare the enum to an
integer with a value, which we shouldn't be able to store in the enumeration in
the first place. "var = 1;" produces as expected an error, "invalid conversion
from ‘int’ to ‘tEnumType’".
> Also, that would warn for perfectly valid (and very common) uses of
> enumeration
> types for bitmasks, e.g. std::ios::openmode could be defined as an enumeration
> type and you could say
>
>if (mode == (std::ios::in|std::ios::out))
>
> where there is no enumerator defined for in|out, but this code is common and
> should not warn.
>
Just because it is common, it doesn't mean that it's perfect. In my opinion
this is an abuse of the enumeration here. If this is a bitmask, it should be
implemented this way, using either the c-style unions or c++ bitsets or
something else (custom class, ...). The name enumeration means we are counting
something, not a bitmask.
E.g. "mode = (std::ios::in|std::ios::out)" only works, because the function "|"
is defined using static_cast. For me this is quite a hint, that it's quite not
perfect, but more an abuse.
> > If it was defined (e.g. typedef enum {NONE = 0, ONE = 1, TWO = 2, THREE = 3}
> > tEnumType), result could again be of the enumeration type and there would
> > be no
> > warning.
>
> No, the result would still be an int, ONE|TWO has type int, period.
>
> I think the warning could be useful in some cases, but it needs to be defined
> much more carefully than simply "warning each time a enumeration type is
> compared to a non enumeration type" as you suggested.
I still think the warning should be there in both cases. It shouldn't be a
default warning, as legacy code would produce too many warnings.
For my solution, from now on I will use "enum class", as it does exactly what I
want. While this is not my optimal solution (forcing at least gcc 4.4 for all
project attendees), it will work, somehow.
