Why does Clang/LLVM warn me about using default in a switch statement where all enumerated cases are covered?

Question

Consider the following enum and switch statement:

typedef enum {
 MaskValueUno,
 MaskValueDos
} testingMask;
void myFunction(testingMask theMask) {
 switch (theMask) {
 case MaskValueUno: {}// deal with it
 case MaskValueDos: {}// deal with it
 default: {} //deal with an unexpected or uninitialized value
 }
};

I'm an Objective-C programmer, but I've written this in pure C for a wider audience.

Clang/LLVM 4.1 with -Weverything warns me at the default line:

Default label in switch which covers all enumeration values

Now, I can sort of see why this is there: in a perfect world, the only values entering in the argument theMask would be in the enum, so no default is necessary. But what if some hack comes along and throws an uninitialized int into my beautiful function? My function will be provided as a drop in library, and I have no control over what could go in there. Using default is a very neat way of handling this.

Why do the LLVM gods deem this behaviour unworthy of their infernal device? Should I be preceding this by an if statement to check the argument?

• Swizzlr: I should say, my reason for -Weverything is to make myself a better programmer. As the NSHipster says: "Pro tip: Try setting the -Weverything flag and checking the "Treat Warnings as Errors" box your build settings. This turns on Hard Mode in Xcode.".
• Steven Fisher: -Weverything can be useful, but be careful about mutating your code too much to deal with it. Some of those warnings are not only worthless but counter-productive, and are best turned off. (Indeed, that's the use case for -Weverything: start with it on, and turn off what doesn't make sense.)
• Steven Fisher: Could you expand a bit on the warning message for posterity? There's usually more to them than that.
• bbaja42: After reading the answers, I still prefer your initial solution. Usage of default statement IMHO is better than the alternatives given in the answers. Just a small note: the answers are really good and informative, they show a cool way how to solve the problem.
• Swizzlr: @StevenFisher That was the entire warning. And as Killian pointed out if I modify my enum later it would open the possibility for valid values to pass to the default implementation. It seems to be a good design pattern (if it is such a thing).

Answer 1

Here's a version that suffers from neither the problem clang's reporting or the one you're guarding against:

void myFunction(testingMask theMask) {
 assert(theMask == MaskValueUno || theMask == MaskValueDos);
 switch (theMask) {
 case MaskValueUno: {}// deal with it
 case MaskValueDos: {}// deal with it
 }
}

Killian has explained already why clang emits the warning: if you extended the enum, you'd fall into the default case which probably isn't what you want. The correct thing to do is to remove the default case and get warnings for unhandled conditions.

Now you're concerned that someone could call your function with a value that's outside the enumeration. That sounds like failing to meet the function's prerequisite: it's documented to expect a value from the testingMask enumeration but the programmer has passed something else. So make that a programmer error using assert() (or NSCAssert() as you said you're using Objective-C). Make your program crash with a message explaining that the programmer is doing it wrong, if the programmer does it wrong.

• Josh Kelley: +1. As a small modification, I prefer to have each case return; and add an assert(false); to the end (instead of repeating myself by listing the legal enums in an initial assert() and in the switch).
• user29079: What if the incorrect enum isn't passed by a dumb programmer, but rather by a memory corruption bug? When the driver presses the break of their Toyota and a bug has corrupted the enum, then the break-handling program should crash & burn, and there should be a text displayed to the driver: "the programmer is doing it wrong, bad programmer!". I don't quite see how this will help the user, before they drive over the edge of a cliff.
• user4051: @Lundin is that what the OP is doing, or is that a pointless theoretical edge case you've just constructed? Regardless, a "memory corruption bug" [i] is programmer error, [ii] is not something you can continue from in a meaningful way (at least not with the requirements as stated in the question).
• user29079: @GrahamLee I'm just saying that "lay down and die" isn't necessarily the best option when you spot an unexpected error.
• Stack Exchange Broke The Law: It has the new problem that you might let the assertion and the cases get out of sync accidentally.

Answer 2

Having a default label here is an indicator that you're confused about what you're expecting. Since you have exhausted all possible enum values explicitly, the default cannot possibly be executed, and you don't need it to guard against future changes either, because if you extended the enum, then the construct would already generate a warning.

So, the compiler notices that you have covered all bases but appear to be thinking that you haven't, and that is always a bad sign. By taking the minimal effort to change the switch to the expected form, you demonstrate to the compiler that what you appear to be doing is what you are actually doing, and you know it.

• Swizzlr: But my concern is a dirty variable, and guarding against that (such as, like I said, an uninitialized int). It seems to me that it would be possible for the switch statement to reach default in such a situation.
• Kilian Foth: I don't know the definition for Objective-C by heart, but I assumed that a typdef'd enum would be enforced by the compiler. In other words, an "uninitialized" value could only enter the method if your program already exhibits undefined behaviour, and I find it entirely reasonable that a compiler doesn't take that into account.
• user4051: @KilianFoth no, they aren't. Objective-C enums are C enums, not Java enums. Any value from the underlying integral type could be present in the function argument.
• user29079: Also, enums could be set in runtime, from integers. So they can contain any value imaginable.
• Matthew James Briggs: OP is correct. testingMask m; myFunction(m); would most likely hit the default case.

Answer 3

Clang is confused, having a default statement there is perfectly fine practice, it is known as defensive programming and is considered good programming practice (1). It is used plenty in mission-critical systems, though perhaps not in desktop programming.

The purpose of defensive programming is to catch unexpected errors that in theory would never happen. Such an unexpected error is not necessarily the programmer giving the function an incorrect input, or even an "evil hack". More likely, it could be caused by a corrupt variable: buffer overflows, stack overflow, runaway code and similar bugs not related to your function could be causing this. And in case of embedded systems, variables could possibly change because of EMI, particularly if you are using external RAM circuits.

As for what do write inside the default statement... if you suspect that the program has gone haywire once you ended up there, then you need some sort of error handling. In many cases you can probably just simply add an empty statement with a comment: "unexpected but doesn't matter" etc, to show that you have given thought to the unlikely situation.

---

(1) MISRA-C:2004 15.3.

• user29079: Please not that the average PC programmer typically finds the concept of defensive programming completely alien, since their view of a program is an abstract utopia where nothing that can be wrong in theory will go wrong in practice. Therefore you will get quite diverse answers to your question depending on who you ask.
• Jens Ayton: Clang is not confused; it has been explicitly asked to provide all warnings, which includes ones that are not always useful, such as stylistic warnings. (I personally opt into this warning because I don't want defaults in my enum switches; having them means I don't get a warning if I add an enum value and don't handle it. For this reason, I always handle the bad value case outside the enum.)
• user29079: @JensAyton It is confused. All professional static analyser tools as well as all MISRA-compliant compilers will give a warning if a switch statement lacks a default statement. Try one yourself.
• Jens Ayton: Coding to MISRA-C is not the only valid use for a C compiler, and, again, -Weverything turns on every warning, not a selection appropriate to a particular use case. Not fitting your taste isn’t the same thing as confusion.
• cHao: @Lundin: I'm pretty sure sticking to MISRA-C does not magically make programs safe and bug-free...and i'm equally sure that there is plenty of safe, bug-free C code from people who have never even heard of MISRA. In fact, it's little more than someone's (popular, but not undisputed) opinion, and there are people who find some of its rules arbitrary and sometimes even harmful outside of the context for which they were designed.

Answer 4

Better still:

typedef enum {
 MaskValueUno,
 MaskValueDos,
 MaskValue_count
} testingMask;
void myFunction(testingMask theMask) {
 assert(theMask >= 0 && theMask<MaskValue_count);
 switch theMask {
 case MaskValueUno: {}// deal with it
 case MaskValueDos: {}// deal with it
 }
};

This is less error-prone when adding items to the enum. You can skip the test for>= 0 if you make your enum values unsigned. This method only works if you have no gaps in your enum values, but that is often the case.

• Martin Sugioarto: This is polluting the type with values you simply don't want that the type contains. It has similarities with the good old WTF here: thedailywtf.com/articles/What_Is_Truth_0x3f_

Answer 5

But what if some hack comes along and throws an uninitialized int into my beautiful function?

Then you get Undefined Behavior, and your default will be meaningless. There's nothing that you can possibly do to make this any better.

Let me be more clear. The instant someone passes an uninitialized int into your function, it is Undefined Behavior. Your function could solve the Halting Problem and it wouldn't matter. It is UB. There is nothing you can possibly do once UB has been invoked.

• jgauffin: How about logging it or throw an exception instead if silently ignore it?
• user29079: Indeed, there is a lot that can be done, such as... adding a default statement to the switch and gracefully handle the error. This is known as defensive programming. It will not only protect against the dumb programmer handing the function incorrect inputs, but also against various bugs caused by buffer overflow, stack overflow, runaway code etc etc.
• DeadMG: No, it won't handle anything. It is UB. The program has UB the moment the function is entered, and the function body can do nothing about it.
• user29079: @DeadMG An enum can have any value that its corresponding integer type in the implementation might have. There is nothing undefined about it. Accessing the contents of an uninitialized auto variable is indeed UB, but the "evil hack" (which is more likely some sort of bug) might as well toss a well-defined value to the function, even though it is not one of the values listed in the enum declaration.

Answer 6

The default statement wouldn't necessarily help. If the switch is over an enum, any value that is not defined in the enum will end up executing undefined behaviour.

For all you know, the compiler can compile that switch (with the default) as:

if (theMask == MaskValueUno)
 // Execute something MaskValueUno code
else // theMask == MaskValueDos
 // Execute MaskValueDos code

Once you trigger undefined behaviour, there is no going back.

• Jens Ayton: Firstly, it’s an unspecified value, not undefined behaviour. This means the compiler may assume some other value that’s more convenient, but it may not turn the moon into green cheese, etc. Secondly, as far as I can tell, that only applies to C++. In C, the range of values of an enum type is the same as the range of values of its underlying integer type (which is implementation-defined, hence must be self-consistent).
• user29079: However, an instance of an enum variable and an enumeration constant do not necessarily need to have the same width and signedness, they are both impl.defined. But I'm pretty sure that all enums in C are evaluated exactly like their corresponding integer type, so there is no undefined or even unspecified behavior there.

Answer 7

I also prefer to have a default: in all cases. I'm late to the party as usual, but... some other thoughts that I didn't see above:

• The particular warning (or error if also throwing -Werror) is coming from -Wcovered-switch-default (from -Weverything but not -Wall). If your moral flexibility allows you to turn off certain warnings (i.e., excise some things from -Wall or -Weverything), consider throwing -Wno-covered-switch-default (or -Wno-error=covered-switch-default when using -Werror), and in general -Wno-... for other warnings you find disagreeable.
• For gcc (and more generic behavior in clang), consult gcc manpage for -Wswitch, -Wswitch-enum, -Wswitch-default for (different) behavior in similar situations of enumerated types in switch statements.

• I also don't like this warning in concept, and I don't like its wording; to me, the words from the warning ("default label ... covers all ... values") suggest that the default: case will be always be executed, such as

  switch (foo) {
   case 1:
   do_something(); 
   //note the lack of break (etc.) here!
   default:
   do_default();
  }
  

  On first reading, this is what I thought you were running into -- that your default: case would always be executed because there's no break; or return; or similar. This concept is similar (to my ear) to other nanny-style (albeit occasionally helpful) commentary that spews forth from clang. If foo == 1, both functions will be executed; your code above has this behavior. I.e., fail to break-out only if you want to possibly keep executing code from subsequent cases! This appears, however, not to be your problem.

At the risk of being pedantic, some other thoughts for completeness:

• I do however think this behavior is (more) consistent with aggressive type checking in other languages or compilers. If, as you hypothesize, some reprobate does attempt to pass an int or something to this function, which is explicitly intending to consume your own specific type, your compiler should protect you equally well in that situation with an aggressive warning or error. BUT it doesn't! (That is, it seems that at least gcc and clang don't do enum type-checking, but I hear that icc does). Since you aren't getting type-safety, you could get value-safety as discussed above. Otherwise, as suggested in TFA, consider a struct or something that can provide type-safety.
• Another workaround could be creating a new "value" in your enum such as MaskValueIllegal, and not supporting that case in your switch. That would be eaten by the default: (in addition to any other wacky value)

Long live defensive coding!

Answer 8

Here's an alternative suggestion:
The OP is trying to protect against the case where someone passes in an int where an enum is expected. Or, more likely, where someone has linked an old library with a newer program using a newer header with more cases.

Why not change the switch to handle the int case? Adding a cast in front of the value in the switch eliminates the warning and even provides a degree of hint about why the default exists.

void myFunction(testingMask theMask) {
 int maskValue = int(theMask);
 switch(maskValue) {
 case MaskValueUno: {} // deal with it
 case MaskValueDos: {}// deal with it
 default: {} //deal with an unexpected or uninitialized value
 }
}

I find this much less objectionable than the assert() testing each of the possible values or even making the assumption that the range of enum values is well-ordered so that a simpler test works. This is just an ugly way of doing what default does precisely and beautifully.

• gnat: this post is rather hard to read (wall of text). Would you mind editing it into a better shape?