Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Mon, 7 Sep 2020 at 23:53, Bruce Korb via Gcc wrote: > > On Mon, Sep 7, 2020 at 3:45 PM Florian Weimer wrote: > > > > * Bruce Korb via Gcc: > > > > > I don't write a lot of code anymore, but this sure seems like a > > > gratuitous irritation to me. I've been using > > > > > > // FALLTHRU and > > > // FALLTHROUGH > > > > > > for *DECADES*, so it's pretty incomprehensible why the compiler should > > > have to invalidate my code because it thinks a different coding > > > comment is better. > > > > It's not clear what you are talking about. > > > > Presumably you placed the comment before a closing brace, and not > > immediately before the subsequent case label? > > Nope. I had /* FALLTHROUGH */ on the line before a blank line before > the case label. After Googling, I found an explicit reference that you > had to spell it: // fall through > I did that, and it worked. So I'm moving on, but still ... The canonical reference is https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough and it says FALLTHROUGH is fine (except with -Wimplicit-fallthrough=5 which "doesn’t recognize any comments as fallthrough comments, only attributes disable the warning").
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Tue, Sep 8, 2020 at 2:33 AM Jonathan Wakely wrote: > > Nope. I had /* FALLTHROUGH */ on the line before a blank line before > > the case label. After Googling, I found an explicit reference that you > > had to spell it: // fall through > > I did that, and it worked. So I'm moving on, but still ... > > The canonical reference is > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough > and it says FALLTHROUGH is fine (except with -Wimplicit-fallthrough=5 > which "doesn’t recognize any comments as fallthrough comments, only > attributes disable the warning"). Hi, Thank you. It turns out it was in someone else's code that I'd incorporated into my project. The fall through comment was polluted with a colon that I hadn't noticed, as in: /* FALLTHROUGH: */ and your fall through regex doesn't allow for that. I'd add a colon to the space, tab and '!' that the regex accepts. Does your acceptance pattern accept these? It's hard for me to decipher. getdefs/getdefs.c:/* FALLTHROUGH */ /* NOTREACHED */ agen5/defLex.c:/* FALLTHROUGH */ /* to Invalid input char */ I'd also recommend a modified error message that includes mention of the approved comment. Thank you. Regards, Bruce
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Tue, Sep 08, 2020 at 07:28:45AM -0700, Bruce Korb via Gcc wrote: > On Tue, Sep 8, 2020 at 2:33 AM Jonathan Wakely wrote: > > > Nope. I had /* FALLTHROUGH */ on the line before a blank line before > > > the case label. After Googling, I found an explicit reference that you > > > had to spell it: // fall through > > > I did that, and it worked. So I'm moving on, but still ... > > > > The canonical reference is > > https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wimplicit-fallthrough > > and it says FALLTHROUGH is fine (except with -Wimplicit-fallthrough=5 > > which "doesn’t recognize any comments as fallthrough comments, only > > attributes disable the warning"). > > Thank you. It turns out it was in someone else's code that I'd > incorporated into my project. > The fall through comment was polluted with a colon that I hadn't noticed, as > in: > > /* FALLTHROUGH: */ > > and your fall through regex doesn't allow for that. > I'd add a colon to the space, tab and '!' that the regex accepts. I think it is a bad idea to change the regexps, it has been done that way for quite a while and many people could rely on what exactly is and is not handled. You can either adjust your code, or switch to using -Wimplicit-fallthrough=2 if the project wants the comments to be even more free form than what the default handles. The regexps came from extensive code searches for what is used in the wild. There is always the option to use attributes or builtins... Jakub
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Tue, Sep 8, 2020 at 7:36 AM Jakub Jelinek wrote: > > The fall through comment was polluted with a colon that I hadn't noticed, > > as in: > > > > /* FALLTHROUGH: */ > > > > and your fall through regex doesn't allow for that. > > I'd add a colon to the space, tab and '!' that the regex accepts. > > I think it is a bad idea to change the regexps, it has been done that way > for quite a while and many people could rely on what exactly is and is not > handled. That is your call. I've never used the colon myself, but my friend did in this example. Unfortunately, I don't get to pick what compiler options folks like to pick for building my code, so I cannot choose the fallthrough level of 2. Anyway, the error message ought to include enough information that folks can fix it without having to resort to multiple Google searches and reading discussions. (Just mention "// fall through" in the message.) > There is always the option to use attributes or builtins... Not if you're writing code for multiple platforms. :( Thank you. Regards, Bruce
Re: Why was it important to change "FALLTHROUGH" to "fall through"?
On Tue, 8 Sep 2020 at 17:33, Bruce Korb wrote: > > On Tue, Sep 8, 2020 at 7:36 AM Jakub Jelinek wrote: > > > The fall through comment was polluted with a colon that I hadn't noticed, > > > as in: > > > > > > /* FALLTHROUGH: */ > > > > > > and your fall through regex doesn't allow for that. > > > I'd add a colon to the space, tab and '!' that the regex accepts. > > > > I think it is a bad idea to change the regexps, it has been done that way > > for quite a while and many people could rely on what exactly is and is not > > handled. > > That is your call. I've never used the colon myself, but my friend did > in this example. Unfortunately, I don't get to pick what compiler > options folks like to pick for building my code, so I cannot choose > the fallthrough level of 2. > > Anyway, the error message ought to include enough information that > folks can fix it without having to resort to multiple Google searches > and reading discussions. (Just mention "// fall through" in the > message.) I don't think "just" doing that will help. If you have a comment that says "fall through:" (with a colon) and the compiler says 'try adding "fall through"' (without a colon) users will just get angry at it. "I did add that, dammit!" etc.
Re: git hooks update
The new version of the hooks is now in use for the production repository. -- Joseph S. Myers jos...@codesourcery.com
