On Jan 21, 2012, at 11:16 AM, David Blaikie wrote: > On Sat, Jan 21, 2012 at 11:02 AM, Fariborz Jahanian <[email protected]> > wrote: >> >> On Jan 21, 2012, at 10:12 AM, David Blaikie wrote: >> >>> Author: dblaikie >>> Date: Sat Jan 21 12:12:07 2012 >>> New Revision: 148640 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=148640&view=rev >>> Log: >>> Add -Wswitch-enum-redundant-default. >>> >>> This warning acts as the complement to the main -Wswitch-enum warning (which >>> warns whenever a switch over enum without a default doesn't cover all >>> values of >>> the enum) & has been an an-doc coding convention in LLVM and Clang in my >>> experience. The purpose is to ensure there's never a "dead" default in a >>> switch-over-enum because this would hide future -Wswitch-enum errors. >> >> We have a related problem with -Wswitch-enum in clang. with this option a >> default should not turn >> off the warning when default covers un-used case labels. Currently clang's >> behavior of --Wswitch and >> -Wswitch-enum are the same. I think fixing that will not make the new >> option necessary. >> >> -Wswitch >> Warn whenever a "switch" statement has an index of enumerated type >> and lacks a "case" for one or >> more of the named codes of that enumeration. (The presence of a >> "default" label prevents this >> warning.) "case" labels outside the enumeration range also >> provoke warnings when this option is >> used. This warning is enabled by -Wall. >> >> -Wswitch-enum >> Warn whenever a "switch" statement has an index of enumerated type >> and lacks a "case" for one or >> more of the named codes of that enumeration. "case" labels >> outside the enumeration range also >> provoke warnings when this option is used. >> >> >> int test18() { >> enum { A, B } a; >> switch (a) { >> case A: return 0; >> default: return 2; >> } >> } > > I'm not entirely sure I follow you - you mean that -Wswitch-enum > should warn whenever there's a switch-over-enum that doesn't > explicitly list all the enum values, even in the presence of a > default? That seems like it would be very noisy/less valuable. There's > often cases where one wants to handle an explicit subset of enum > values & ignore the rest (granted, this case does devalue the switch > warning). > > I was wondering what the difference between -Wswitch and -Wswitch-enum > was, but when I read the GCC docs it wasn't clear to me. > > Now that I search again, the docs I found are more explicit and > support your statement: > > -Wswitch > Warn whenever a switch statement has an index of enumerated type > and lacks a case for one or more of the named codes of that > enumeration. (The presence of a default label prevents this warning.) > case labels outside the enumeration range also provoke warnings when > this option is used (even if there is a default label). This warning > is enabled by -Wall. > > -Wswitch-enum > Warn whenever a switch statement has an index of enumerated type > and lacks a case for one or more of the named codes of that > enumeration. case labels outside the enumeration range also provoke > warnings when this option is used. The only difference between > -Wswitch and this option is that this option gives a warning about an > omitted enumeration code even if there is a default label.
Yes, clang currently treats -Wswitch-enum same as -Wswitch. Maybe, while you are at it you can address this too. Once -Wswitch-enum is corrected, a "default' can no longer hide missing case labels. > > http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html > > So this would seem to say that the new warning I added is applicable > to both -Wswitch and -Wswitch-enum. It's probably less valuable in the > latter case - since, if you're using -Wswitch-enum, you don't need to > worry about the presence or absence of 'default' labels as much. They > might be unreachable code but they won't hide future mistakes. Yes. - Thanks, Fariborz > > So to correct my statement in the patch, > -Wswitch-enum-redundant-default is important for -Wswitch for those > reasons, but not important for -Wswitch-enum because the latter will > catch you regardless of the default. > > (excuse the rambling reply - I think I found the right conclusion, though ;)) > > - David _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
