-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Many implicit-fallthrough errors #7295
Comments
Technically, they're implicit-fallthrough warnings. It's your configuration
that's treating them as errors. That's probably not a good idea for third-party
source code over which you don't have direct control.
|
Are not all of these places marked with |
Not long ago the Ref with some details: https://www.kernel.org/doc/html/v5.7/process/deprecated.html#implicit-switch-case-fall-through |
(What a weird behavior-breaking change.) So we'll need both the comment and a new creative way to define that attribute for the compilers that insist on that method... |
@yelkarama, are you interested in working on a fix for this? |
I'd expect this (WIP) page to list such a breaking change. I also was unable to locate a commit that may introduce such change. Maybe it's some kind of bug in gcc (the gcc12 cycle has just begun)? |
Make configure/cmake check if the attribute works, then use the FALLTHROUGH define instead of the previous comment to inhibit compiler warnings for when a switch-case doesn't end with a break or return. Reported-by: Younes El-karama Fixes #7295
I did a first take on using the attribute - in #7302 |
[ Looks like this feature first appeared in GCC 7 with support for the attribute and various flavours of comments, configurable via the |
Based on the results of my attempt to fix this, I think we might instead rather decide to stick with the comment version for the time being. There doesn't even seem to be a reliable way to silence these warnings using the attribute method in our CI, so I can't even start to fathom how spectacularly it'll fail at users' places if we'd decide to ship that. @yelkarama I propose you tell your compiler to accept the comments as fallthrough-indicators. |
... since it no longer acknowledges the comment markup we use for that purpose. Reported-by: Younes El-karama Fixes #7295
@bagder: It indeed looks like a hot mess. Tried digging up info about the default warning mode in GCC, but the documentation wasn't helpful there. The LLVM doc is even less talkative and for some reason lists this option as far back as clang 4.0.0 (early 2017). The end result when doing an actual build is rather accidental. It's less than ideal because otherwise this warning (when justified) is useful. |
The warning is indeed very useful, but fortunately we can probably still enjoy the warnings from pre gcc-12 compilers for a fairly long time ahead and hopefully things get fixed in 12 or later in the mean time! |
@bagder Sounds good to me! Meanwhile, if somebody has some definitive info/pointer on how to best deal with this issue, I guess we'd all love to learn about it. |
Commit b5a434f inhibits the warning on implicit fallthrough cases, since the current coding of indicating fallthrough with comments is falling out of fashion with new compilers. This attempts to make the issue smaller by rewriting fallthroughs to no longer fallthrough, via either breaking the cases or turning switch statements into if statements. lib/content_encoding.c: the fallthrough codepath is simply copied into the case as it's a single line. lib/http_ntlm.c: the fallthrough case skips a state in the state- machine and fast-forwards to NTLMSTATE_LAST. Do this before the switch statement instead to set up the states that we actually want. lib/http_proxy.c: the fallthrough is just falling into exiting the switch statement which can be done easily enough in the case. lib/mime.c: switch statement rewritten as if statement. lib/pop3.c: the fallthrough case skips to the next state in the statemachine, do this explicitly instead. lib/urlapi.c: switch statement rewritten as if statement. lib/vssh/wolfssh.c: the fallthrough cases fast-forwards the state machine, do this by running another iteration of the switch statement instead. lib/vtls/gtls.c: switch statement rewritten as if statement. lib/vtls/nss.c: the fallthrough codepath is simply copied into the case as it's a single line. Also twiddle a comment to not be inside a non-brace if statement. Closes: #xxxx See-also: curl#7295
Commit b5a434f inhibits the warning on implicit fallthrough cases, since the current coding of indicating fallthrough with comments is falling out of fashion with new compilers. This attempts to make the issue smaller by rewriting fallthroughs to no longer fallthrough, via either breaking the cases or turning switch statements into if statements. lib/content_encoding.c: the fallthrough codepath is simply copied into the case as it's a single line. lib/http_ntlm.c: the fallthrough case skips a state in the state- machine and fast-forwards to NTLMSTATE_LAST. Do this before the switch statement instead to set up the states that we actually want. lib/http_proxy.c: the fallthrough is just falling into exiting the switch statement which can be done easily enough in the case. lib/mime.c: switch statement rewritten as if statement. lib/pop3.c: the fallthrough case skips to the next state in the statemachine, do this explicitly instead. lib/urlapi.c: switch statement rewritten as if statement. lib/vssh/wolfssh.c: the fallthrough cases fast-forwards the state machine, do this by running another iteration of the switch statement instead. lib/vtls/gtls.c: switch statement rewritten as if statement. lib/vtls/nss.c: the fallthrough codepath is simply copied into the case as it's a single line. Also twiddle a comment to not be inside a non-brace if statement. Closes: #7322 See-also: #7295 Reviewed-by: Daniel Stenberg <daniel@haxx.se>
I did this
Building the latest release of curl (7.77) on RHEL6 using newly built gcc toolchain (git: 12.0.0)
When using Werror, I'm getting these many implicit-fallthrough error:
I expected the following
Successful build
curl/libcurl version
7.77
operating system
Linux localhost.localdomain 2.6.32-754.35.1.el6.x86_64 #1 SMP Sat Nov 7 12:42:14 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
The text was updated successfully, but these errors were encountered: