Some style remarks: typedef isn't needed in C++, and why not just static_cast<int> your enum value? No need for a special function there. The reverse is more tricky of course, because without a c++0x enum class you won't be sure what you're doing is correct.
–
rubenvbJun 16 '11 at 18:51

@Beta, @ulidtko: not undefined behaviour - the value is cast to the underlying integral type that the compiler's selected to store the enum (typically int these days, but there are rules in the Standard about how alternative types should be selected if some enumeration values can't fit in that type). Hopefully a compiler would warn if the value you provided was outside the range the enum's underlying integral type could store, but even then I'd expect it to chop the value into range much like char(2873) would.
–
Tony DJun 17 '11 at 1:02

4 Answers
4

The problem is that a variable of type Test can have any value allowed by the type the compiler gives it. So if it decided it was a 32-bit unsigned integer, any value in that range is allowed. So, if for instance you call foo(123456), your switch statement will not catch any value and there's no return after your switch.

@SolarBear: do not put a default in a switch just to silence a warning. Having switches over enum without default is good practice, since it means that whenever you update the enum, you will be warn if you forgot to update a switch. You can, however, put a call to abort after the switch proper.
–
Matthieu M.Jun 16 '11 at 18:49

2

@ulidtko: you can call foo(static_cast<Test>(123456));, which is basically the same. Think about translating back an integer that came through a network packet into the enum, you have to perform such casts, and without proper validation, they are harmful :/
–
Matthieu M.Jun 16 '11 at 18:51

2

@Beta: 1) if a switch wasn't handled the warning would be "enumeration value XXX not handled in switch", but all the declared values are handled - great. 2) the warning in the question is "control reaches end of non-void function"... to address that warning you should NOT add a default to the switch, but an appropriate compilation/flow-control statement after the switch. (I've even verified both these aspects with g++ 4.5.2 -Wall.)
–
Tony DJun 17 '11 at 3:21

2

@Beta: Tony answered for me, you and I not talking about the same warning :) I have a macro UNREACHABLE(XXX) which expects a c-string and expands to assert(!XXXX) (in debug) and throw Unreachable(XXX) (in release). In your case, I would put it at the bottom of the function (after the switch) to both silence gcc's warning and be notified in case it is reached. And I would still not put a default statement :)
–
Matthieu M.Jun 17 '11 at 6:28

1

@Tony, @Matthieu M.: Tony makes a very good point. (My compiler is out of date.) I'd like to sleep on it, but in the light of this I'm inclined to agree: the catch-all belongs after the switch.
–
BetaJun 17 '11 at 17:07

Although there is actual danger of passing foo an argument that will not hit any of the return statements, the warning does not depend on enum, or on the danger. You can see the same effect with bool, in a switch statement which is (as far as I can tell) completely watertight.

In general the compiler isn't smart enough to deduce whether you've covered every possible path that control could actually take through the switch statement. To be that smart it would have to be able to deduce all of the possible states the program can reach before entering the switch, which leads straight to the Halting Problem.

So the deduction has to stop somewhere, and (at least with gcc) it stops with the determination that there is no default case and that therefore control might be able to leave the switch without hitting return.

"You can see the same effect with bool"... couldn't reproduce under gcc 4.5.2 but could under 4.1.2. Time for a compiler upgrade.
–
Tony DJun 17 '11 at 3:27

@Tony: Excellent point. The deduction still has to stop somewhere, but it sounds as if 4.5.2 takes it farther than 4.1.2. I think a watertight case exists which 4.5.2 would be unsure about, but it would be more contrived. Yes, time for me to update my compiler...
–
BetaJun 17 '11 at 17:04

Isn't this Undefined Behaviour? Also see edit.
–
ulidtkoJun 16 '11 at 18:53

1

The underlying type must be wide enough to hold all the bits of the enum values, and values using those bits are valid. So Test(a|b|c) is a valid value (that happens to be Test(3)).
–
Bo PerssonJun 16 '11 at 19:17

2

@ulidtko: since C++ has no explicit range checking it's relatively easy for an enum to contain an invalid value - you should code defensively so that you catch such a case rather than letting it just fall through the switch and thereby return an undefined value from the function.
–
Paul RJun 16 '11 at 19:20

@Bo Persson: "must be wide enough"... true, but that might be misinterpreted as meaning it can't be even wider... (of course, it can be - many compilers would use int even for the Test above, which is good because it's more interoperable with C).
–
Tony DJun 17 '11 at 0:57

Although your enum has only three declared states, the implementation will actually choose a larger integral type (typically int) to store the values, which are NOT restricted to those declared. The Standard just makes some minimal guarantees to ensure it can at least handle the values specified and certain combinations. Sometimes this freedom is essential, as the enum values are intended to be bitwise-ORed together to form combinations. So, the function foo can actually be called with say, Test(3), which wouldn't find a return statement in your switch.

The | causes conversion of the enums to ints before the | operator kicks in, so you need to cast it back to Test before calling foo: foo(Test(a | b)). You can also have a (non-member obviously) function Test operator|(Test lhs, Test rhs) { return Test(lhs | rhs); } so avoid the inconvenience if your enums are intended to be used in that way.
–
Tony DJun 17 '11 at 0:54