Hi Cyril, I agree. Applied in r151709!
Thanks so much, Ted On Feb 28, 2012, at 5:45 PM, Cyril Roelandt <[email protected]> wrote: > 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. > <unreachable_code_default_statement.patch>_______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
