gcc: how best to handle warning about (unreachable) end of function after switch?

Question

When I compile the following code at c++11 standard, it works fine with clang, and also with gcc, but gcc (all versions that I tested 4.8.2, 4.9.2, 5.1.0) gives a warning:

#include <iostream>

enum class FOO { A, B, C };

const char * bar(FOO f) {
  switch (f) {
    case FOO::A:
      return "A";
    case FOO::B:
      return "B";
    case FOO::C:
      return "C";
  }
}

int main() {
  unsigned int x;
  std::cin >> x;

  FOO f = static_cast<FOO>(x % 3);
  std::cout << bar(f) << std::endl;
}

The warning is -Wreturn-type:

main.cpp: In function ‘const char* bar(FOO)’:
main.cpp:14:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1plus: all warnings being treated as errors

I still get the warning even with -O2 or -O3 optimizations -- does this mean that even at high optimization levels, gcc cannot dead-code eliminate the 'end' of the function?

Notably, it doesn't give me the warning about unhandled switch cases.

Edit: From experiments with godbolt, it appears that even at high levels, it doesn't dead code eliminate that. I'm not sure whether it could or if clang does for instance.

Is there a good way to suppress this warning locally within such a function, or is the only way to suppress this to disable the warning generally?

Edit: I guess the question poses a natural language lawyer question, judging from the answers:

Can a conforming compiler dead-code eliminate the "end" of the bar function in my listing? (Or 101010's version with return nullptr; appended?) Or does conforming to the standard require that it generate code to handle enum values that aren't part of the enum definition?

My belief was that it could dead-code eliminate that but you are welcome to prove me wrong.

Answer 1

does this mean that even at high optimization levels, gcc cannot dead-code eliminate the 'end' of the function?

Yes, because it's not dead code.

The standard allows you to call your function with static_cast<FOO>(static_cast<int>(FOO::B) | static_cast<int>(FOO::C)) as the argument. Your switch doesn't handle this.

The reason you don't get a warning about this not being handled in your switch is because the warning would have way too many false positives.

Answer 2

I would add an assert(false) at the end of the function:

#include <cassert>

const char * bar(FOO f) {
  switch (f) {
    case FOO::A:
      return "A";
    case FOO::B:
      return "B";
    case FOO::C:
      return "C";
  }

  assert(false);
}

(Note that adding it as the default case wouldn't be as good, since that would silence all unhandled switch case warnings when e.g. adding new enum values.)

This is also good for debugging because it lets you instantly know when bar receives an invalid argument for some reason.

Answer 3

In order to suppress the warning change your code to:

const char * bar(FOO f) {
  switch (f) {
    case FOO::A:
      return "A";
    case FOO::B:
      return "B";
    case FOO::C:
      return "C";
  }

  return nullptr;
  ^^^^^^^^^^^^^^^
}

Or:

const char * bar(FOO f) {
  switch (f) {
    case FOO::A:
      return "A";
    case FOO::B:
      return "B";
    case FOO::C:
      return "C";
    default:
      return nullptr;
      ^^^^^^^^^^^^^^^
  }
}

The compiler warns you that you're not returning anything in the case where f doesn't fall to one of the cases provided in switch. Falling of the end of a non-void function without returning introduces undefined behavior.

the end of your bar function is not unreachable. If I alter the code to:

#include <iostream>

enum class FOO { A, B, C };

const char * bar(FOO f) {
  switch (f) {
    case FOO::A:
      return "A";
    case FOO::B:
      return "B";
    case FOO::C:
      return "C";
  }

  std::cout << "End Reached" << std::endl;
  //return nullptr;
}

int main() {
  unsigned int x = 10;
  FOO f = static_cast<FOO>(x);
  std::cout << bar(f) << std::endl;
}

Output:

End Reached

Live Demo

Answer 4

Surely there are more than one "best way". This answer approaches the problem by eliminating switch whatsoever.

_{...and this particular way might be not the one you expect. I'm risking to be downvoted, but I'm still posting this. The bottom line is: sometimes "paradigm shift" solution works better in the given application than any workarounds and warning disabling.}

switch is a well-known "code smell". I don't say it's always bad, but sometimes you can do better than switch, and it worth to consider alternatives.

In our case, bar function does translation (in linguistic sense). It looks up for some key in the dictionary and returns a translated word. You could explicit your code by using standard containers. The classic example is std::map (which is called "dictionary" in other programming languages):

#include <map>

enum class FOO {
    A, B, C
};

// Might be a vector or even static array if your enumeration is contiguous
std::map<FOO, std::string> dict = {
    { FOO::A, "A" }, // maps ints to strings
    { FOO::B, "B" },
    { FOO::C, "C" },
};

int main() {
    auto it = dict.find(FOO::A);
    if (it == dict.cend()) {
        // Handle "no such key" case
    }
    auto a = it->second; // "A"
}

No matter which solution you choose, the problem arises if you don't have some key in the dictionary. You should handle this case anyway.