[cfe-commits] [scan-build][PATCH 1/1] Unreachable code checker : false positive in switch statements ?

[Cyril Roelandt]

Hello,

While trying out the UnreachableCode checker of scan-build, I noticed 
that it would report an unreachable statement in the following code :

enum foobar { FOO, BAR };
extern void error();
void test(enum foobar fb) {
   switch (fb) {
     case FOO:
       break;
     case BAR:
       break;
     default:
       error();
   }
}


enum.c:10:7: warning: This statement is never executed
       error();
       ^~~~~

Here, scan-build seems to be right. In the test() function, the argument 
"fb" can only have two different values (FOO and BAR), which means we 
should never get into the "default" case. Still, many programmers will 
use this default case, for three reasons :

1) A switch statement without a default case looks weird : it is an 
uncommon pattern.
2) One could call test() with an argument that is neither FOO, neither 
BAR, like test(42). Here, the default case allows programmers to easily 
catch errors : it is more elegant than writing if (fb != FOO && fb != 
BAR) { ... } at the beginning of the test() function.
3) If a "FOOBAR" field is ever added to the "enum foobar", then all 
functions using variables of type "enum foobar" (such as test()) will 
have to be updated. If programmers forget one, the program will end up 
in the default case and trigger an error. Keeping the default case is an 
easy way to find such mistakes.

For all these reasons, I think that "error();" should not be reported as 
an unreachable statement, for it is probably a false positive.

Obviously, in the following code :

default:
   foo();
   break;
   bar();

"bar();" should be reported as an unreachable statement.


I have updated the tests to take this into account, and written a very 
simple patch that seems to do the job. I am not sure it is the best way 
to go, but it should illustrate my point and was not too hard to write 
for an unexperienced clang hacker like me :) I would like to hear what 
you think about it. The whole thing comes attached as a patch.


Looking forward to hearing from you,
Cyril Roelandt.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unreachable_code_default_statement.patch
Type: text/x-diff
Size: 1628 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120229/62ebd3f3/attachment.patch>