On Tue, Aug 30, 2016 at 07:38:18AM -0400, Eric Gallager wrote:
> On 8/30/16, Marek Polacek <pola...@redhat.com> wrote:
> > On Mon, Aug 29, 2016 at 09:32:04AM -0400, Eric Gallager wrote:
> >> I tried v6 on my binutils-gdb fork, and it printed A LOT of
> >> warnings...
> >
> > BTW, why is that so? Does binutils-gdb not use various FALLTHRU comments?
> >
> > Marek
> >
>
>
> There are a lot of comments mentioning fallthroughs, but they mostly
> need to be rewritten to be recognized properly. There's just so many
> different ways of writing them. For example, one I've seen a lot
> writes it as "Drop through" instead of "Fall through" or something.
> Other examples of comments mentioning fallthroughs:
>
> /* else fall through */
> /* else fallthrough to: */
> /* FALL THRU into number case. */
> /* ObjC NSString constant: fall through and parse like STRING. */
> /* Fall through, pretend it is global. */
> /* Fall through into normal member function. */
> /* fall in for static symbols that do NOT start with '.' */
> /* >>> !! else fall through !! <<< */
> /* ... fall through for unsigned ints ... */
> /* fall thru to manual case */
Thanks, this is interesting. I wonder if we should also recognize
"else fall through"; I've seen that in our codebase a few times.
But why would anyone write something as ugly as ">>> !! else fall through !!
<<<"
is beyond me.
> Also, at second glance, it's actually not quite as many warnings as I
> thought it was at first, it mostly just looked that way since each
> -Wimplicit-fallthrough warning takes up 12 lines due to the
> double-fixits. FWIW, Aldy's -Walloca-larger-than patch actually was
> noisier in the GDB portion at least. (The Binutils portion was already
> clean on its alloca usage since it already used the -Wstack-usage
> warning.)
That's nice to hear. The latest version of the patch doesn't have the fix-it
hints, it just says in an inform note where a statement falls through to,
so it's going to be less verbose.
> Oh, and one other thing I discovered:
> __attribute__((fallthrough)) triggers -Wdeclaration-after-statement:
>
> In file included from ./defs.h:124:0,
> from ./cli/cli-setshow.c:20:
> ./cli/cli-setshow.c: In function ‘do_setshow_command’:
> ./../include/ansidecl.h:505:33: warning: ISO C90 forbids mixed
> declarations and code [-Wdeclaration-after-statement]
> # define ATTRIBUTE_FALLTHROUGH __attribute__((fallthrough))
> ^
> ./cli/cli-setshow.c:359:4: note: in expansion of macro ‘ATTRIBUTE_FALLTHROUGH’
> ATTRIBUTE_FALLTHROUGH;
> ^~~~~~~~~~~~~~~~~~~~~
>
> Which doesn't really make sense to me.
Yes, I think this is a bug. I'm pondering how to fix this.
Thanks a lot Eric!
Marek
