no return in function using switch statement

Viewed 2843

I'm devoloping an application in LINUX with an older gcc version (7.something if I remember correctly). Recently I tried to run the same application on Windows. On Windows, I'm using MinGW as a compiler (with gcc 8.1.0) .

I came across this error message while compiling my application on Windows:

warning: control reaches end of non-void function [-Wreturn-type]

the code is similar to the following:

class myClass {
protected:
    enum class myEnum{
        a,
        b,
    };

    int fun(myClass::myEnum e);
}

and

int myClass::fun(myClass::myEnum e) {
    switch (e){
        case myEnum::a:{
            return 0;
        }
        case myEnum::b:{
            return 1;
        }
    }
}

I understand what the error message means, I'm just wondering why it was never an issue in LINUX.

Is this piece of code really an issue and do I have to add some dummy return statements?

Is there a branch of this function that will lead to no return statement?

4 Answers

This is a shortcoming of g++ static analyzer. It doesn't get the fact that all enum values are handled in the switch statement correctly.

You can notice here https://godbolt.org/z/LQnBNi that clang doesn't issue any warning for the code in it's current shape, and issues two warnings ("not all enum values are handled in switch" and "controls reach the end on non-void function") when another value is added to the enum.

Keep in mind, that compiler diagnostic is not standardized in any way - compiler are free to report warnings for conforming code, and report warnings (and compile!) for a malformed program.

You have to keep in mind that in C++ enums are not what they are appear to be. They are just ints with some constraints, and can easily assume other values than these indicated. Consider this example:

#include <iostream>

enum class MyEnum {
  A = 1,
  B = 2
};

int main() {
  MyEnum m {}; // initialized to 0
  switch(m) {
    case MyEnum::A: return 0;
    case MyEnum::B: return 0;
  }
  std::cout << "skipped all cases!" << std::endl; 
}

The way around this is to either put a default case with assert(false) as VTT indicated above, or (if you can give everybody the guarantee that no values from outside the indicated set will ever get there) use compiler-specific hint, like __builtin_unreachable() on GCC and clang:

  switch(m) {
    case MyEnum::A: return 0;
    case MyEnum::B: return 0;
    default: __builtin_unreachable();
  }

Firstly, what you describe is a warning, not an error message. Compilers are not required to issue such warnings, and are still permitted to successfully compile your code - as it is technically valid.

Practically, most modern compilers CAN issue such warnings, but in their default configuration they do not. With gcc, the compiler can be optionally configured to issue such warnings (e.g. using suitable command line options).

The only reason this was "never an issue" under linux is because your chosen compiler was not configured (or used with a suitable command line option) to issue the warning.

Most compilers do extensive analysis of the code, either directly (during parsing the source code) or by analysis of some internal representation of that code. The analysis is needed to determine if the code has diagnosable errors, to work out how to optimise for performance.

Because of such analysis, most compilers can and do detect situations that may be problematical, even if the code does not have diagnosable errors (i.e. it is "correct enough" that the C++ standard does not require a diagnostic).

In this case, there are a number of distinct conclusions that the compiler may reach, depending on how it conducts analysis.

  • There is a switch. In principle, code after a switch statement may be executed.
  • The code after the switch reaches the end of the function without a return, and the function returns a value. The result of this is potential undefined behaviour.

If the compiler's analysis gets this far (and the compiler is configured to warn on such things) criteria for issuing a warning are met. Further analysis is then needed if the warning can be suppressed e.g. determine that all possible values of e are represented by a case, and that all cases have a return statement. The thing is, a compiler vendor may elect not to do such analysis, and therefore not suppress warnings, for all sorts of reasons.

  • Doing more analysis increases compilation times. Vendors compete on claims of their compiler being faster, among other things, so NOT doing some analysis is therefore beneficial in getting lower compilation times;
  • The compiler vendor may consider it is better to flag potential problems, even if the code is actually correct. Given a choice between giving extraneous warnings, or not warning about some things, the vendor may prefer to give extraneous warnings.

In either of these cases, analysis to determine that the warning can be suppressed will not be done, so the warning will not be suppressed. The compiler will simply not have done enough analysis to determine that all paths of execution through the function encounter a return statement.

In the end, you need to treat compiler warnings as sign of potential problems, and then make a sensible decision about whether the potential problem is worth bothering about. Your options from here include suppressing the warning (e.g. using a command line option that causes the warning to be suppressed), modifying the code to prevent the warning (e.g. adding a return after the switch and/or default case in the switch that returns).

One should be very careful when omitting return statements. It is an undefined behavior:

9.6.3 The return statement [stmt.return]

Flowing off the end of a constructor, a destructor, or a function with a cv void return type is equivalent to a return with no operand. Otherwise, flowing off the end of a function other than main (6.6.1) results in undefined behavior.

It may be tempting to consider that this code is fine because all the valid enumerator values (in this case in range 0..1 [0..(2 ^ M - 1)] with M = 1) are handled in switch however compiler is not required to perform any particular reachability analysis to figure this out prior to jumping into UB zone.

Moreover, example from SergeyA's answer shows that this kind of code is a straight time bomb:

class myClass {
protected:
    enum class myEnum{
        a,
        b,
        c
    };

    int fun(myClass::myEnum e);
};

int myClass::fun(myClass::myEnum e) {
    switch (e){
        case myEnum::a:{
            return 0;
        }
        case myEnum::b:{
            return 1;
        }
        case myEnum::c:{
            return 2;
        }
    }
}

Just by adding a third enum member (and handling it in switch) the range of valid enumerator values gets extended to 0..3 ([0..(2 ^ M - 1)] with M = 2) and clang happily accepts it without any complaints even though passing 3 into this function will miss the switch because compiler is not required to report UB either.

So the rule of thumb would be to write code in a manner that all paths end either with return throw or [[noreturn]] function. In this particular case I would probably wrote a single return statement with an assertion for unhandled enumerator values:

int myClass::fun(myClass::myEnum e) {
    int result{};
    switch (e){
        case myEnum::a:{
            result = 0;
            break;
        }
        case myEnum::b:{
            result = 1;
            break;
        }
        default:
        {
           assert(false);
           break;
        }
    }
    return result;
}
Related