10

In the following C++ code:

typedef enum { a, b, c } Test;

int foo(Test test) {
    switch (test) {
        case a: return 0;
        case b: return 1;
        case c: return 0;
    }
}

a warning is issued when compiling with -Wall, saying that control reaches end of non-void function. Why?

Edit

Its not generally correct to say that the variable test in the example can contain any value.

foo(12354) does not compile:

> test.cpp:15:14: error: invalid conversion from ‘int’ to ‘Test’
> test.cpp:15:14: error:   initializing argument 1 of ‘int foo(Test)’

because 12354 isn't a valid Test value (though it indeed would be valid in plain C, but it's not in C++).

You sure could explicitly cast an arbitrary integer constant to the enum type, but isn't that considered Undefined Behaviour?

Community
  • 1
  • 1
ulidtko
  • 14,740
  • 10
  • 56
  • 88
  • Although `foo(12345)` may not compile, `foo(Test(12345))` does. (gcc 4.2.1) – Beta Jun 16 '11 at 18:51
  • Some style remarks: `typedef` isn't needed in C++, and why not just `static_cast` 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. – rubenvb Jun 16 '11 at 18:51
  • @rubenvb, I changed an integer constant to your pleasure `;)`. – ulidtko Jun 16 '11 at 18:57
  • 1
    @ulidtko: haha, ok. Point taken `:)` – rubenvb Jun 16 '11 at 18:57
  • @Beta, yes, explicit cast does work. But isn't that considered Undefined Behaviour? – ulidtko Jun 16 '11 at 18:57
  • @ulidtko: Is it? It's something I'd never do, but I don't know whether it causes UB. (If it does, I wonder why the compiler doesn't warn about *that*!) – Beta Jun 16 '11 at 19:07
  • 1
    @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 Delroy Jun 17 '11 at 01:02

4 Answers4

5

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.

Put a default case in your switch or add some error-handling code.

SolarBear
  • 4,534
  • 4
  • 37
  • 53
  • You can't call `foo(123456)`, it doesn't compile. See my edit. – ulidtko Jun 16 '11 at 18:48
  • 6
    @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(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
  • @Matthieu M.: I respectfully disagree. Having such a switch means that you will get the warning *every time you compile*, whether or not you've touched the `enum` type. At best that'll be a distraction, at worst you'll learn to ignore warnings. Just put in a default case that will throw an exception (or `assert(false)`). – Beta Jun 16 '11 at 18:58
  • @Beta: Concur. In my little neck of the programming world, switch statements _always_ have to have a default, even if the default is theoretically unreachable. My little neck of the world is plagued by single event upsets. Cosmic rays don't give a hoot about encapsulation, the valid range of an enum, or lots of other niceties of object oriented programming. A switch statement without a default is going to get hammered big time in a code review. – David Hammen Jun 17 '11 at 00:59
  • @David Hammen: the best error message I ever saw was one I'd written-- to cover a case that could surely never happen, but I was just making my code mathematically tidy. It was to analyze an aircraft skin, and distinguish the inside surface from the outside. On the first one they handed me, *"Warning: Klein bottle topology"* – Beta Jun 17 '11 at 01:16
  • 2
    @Beta: Matthieu's right (as usual)... the current warning is from the function not necessarily hitting a `return` statement... to fix it, there should be some throw, abort, exit, assert, or even return (if a meaningful runtime garbage-handling behaviour makes sense) *after* the switch, not inside the switch as that unnecessarily prevents the compiler from verifying the declared enumerations are all handled. @David: basically there *is* a default behaviour, but it shouldn't be a case inside the switch for the reasons mentioned. – Tony Delroy Jun 17 '11 at 01:24
  • Depends on your project / your environment. I have yet to work on a project where there isn't an explicit or implicit rule that all switch statements must have a default. (The places where the rule is implicit, this is just one of those 'obvious' things that don't need an explicit rule). Whether the rule is explicit or implicit, a comment in a code review "this switch statement has no default" is a big bzzt, wrong. The code _has_ to be fixed. – David Hammen Jun 17 '11 at 02:07
  • @Tony: In the example code, the declared enumerations *are* all handled, yet the warning appears. The verification you refer to does not take place (at least not in gcc 4.2.1). – Beta Jun 17 '11 at 02:53
  • 3
    @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 Delroy Jun 17 '11 at 03:21
  • 3
    @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 06:28
  • 2
    @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. – Beta Jun 17 '11 at 17:07
4

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.

Beta
  • 96,650
  • 16
  • 149
  • 150
  • 2
    "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 Delroy Jun 17 '11 at 03: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... – Beta Jun 17 '11 at 17:04
  • Just to confirm the issue is there also in gcc 4.9.2. But if SolarBear is right and you can effectively have a "unortodox" value in the enum variable, than the compiler warning is a good thing, and we should really go for some error handling in the default case. – Antonio Apr 01 '15 at 16:21
3

There is no guarantee that the variable test will contain a valid enum so it is in fact possible for you to reach the end of your non-void function, e.g. if your calling code looks like this:

Test test = Test(3);
foo(test);
Paul R
  • 208,748
  • 37
  • 389
  • 560
  • 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 Persson Jun 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 R Jun 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 Delroy Jun 17 '11 at 00:57
1

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.

Tony Delroy
  • 102,968
  • 15
  • 177
  • 252
  • I'm getting compile error when trying to do `foo(a | b)`. Also see my edit. – ulidtko Jun 16 '11 at 18:52
  • 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 Delroy Jun 17 '11 at 00:54