-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix some "enumeration values not explicitly handled in switch" warnings #555
Conversation
./src/json.hpp:1968:21: warning: enumeration value 'discarded' not explicitly handled in switch [-Wswitch-enum]
./src/json.hpp:9339:21: warning: enumeration values 'number_integer', 'number_unsigned', and 'number_float' not explicitly handled in switch [-Wswitch-enum]
In this case the switch is useless since the other cases are already excluded by the initial asserts. Removing the switch altogether seems the best way forward.
./src/json.hpp:2821:17: warning: 6 enumeration values not explicitly handled in switch: 'null', 'boolean', 'number_integer'... [-Wswitch-enum]
I'm not sure what you are fixing here, but there are still warnings when compiled with
|
It seems that the compiler emits the warning only when a templated function is actually used. I'm currently using the library just to output a bunch of JSON data and when I compile my program it doesn't emit any warning (because I fixed only the ones I'm interested in). Even more strange is that if I try to compile the following empty test program: #include "src/json.hpp"
int main() {
return 0;
} it emits one warning:
(on line 11693, that is not patched in this PR), even if I expect to not emit any warning... Anyway: there are some other You can merge this patch as is, to fix at least the 4 warnings and leave the remaining for later... or just reject the PR, up to you :-). |
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
|
The output from gcc is different from clang here. Consider this: enum Enum {
ValA, ValB
};
int main() {
Enum x = ValA;
switch (x) { // Line 8
case ValA:
case ValB:
break;
};
switch (x) {
case ValA:
case ValB:
default: // Line 17
break;
};
int y = 0;
switch (y) { // Line 22
case 10:
case 20:
break;
};
return 0;
} and the following calls to compilers:
So it seems that in clang IMHO, to be really useful, gcc's |
Yes, the switch on enum diagnostics are different between gcc and clang. They apparently have different philosophies about switching on an enum. Clang thinks that if you cover all the explicit values, then you should ignore anything else. Gcc thinks that you should have a default case to handle anything not specified. So it is impossible to make code which is clean with both https://clang.llvm.org/docs/DiagnosticsReference.html
|
Thanks for the effort, but I decided not to merge this. As @gregmarr described, you can't have it all ways. |
This change aims to remove the warnings emitted with the
-Wswitch-enum
flag and remove thedefault
case.The motivation for removing the default case is that I want a compiler warning if new values are added to an enumeration and there are switch/case that needs to be updated to handle them.
Moreover in some cases I added an
assert(false)
at the end of the switch/case: this means that an invalid enum value has been passed when this should not happen.