Implicit fallthrough warning on GCC 7.x

[russel]

I have git master up-to-date as at 2017-02-14T08:13+00:00, with Fedora Rawhide fully up to date, and attempt to build Pony results in:

|> make
Makefile:224: WARNING: Unsupported LLVM version: 3.9.0
Makefile:225: Please use LLVM 3.7.1, 3.8.1, or 3.9.1
actor.c
messageq.c
fun.c
src/libponyrt/ds/fun.c: In function ‘siphash24’:
src/libponyrt/ds/fun.c:46:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 7: b |= ((uint64_t)in[6]) << 48;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:47:5: note: here
     case 6: b |= ((uint64_t)in[5]) << 40;
     ^~~~
src/libponyrt/ds/fun.c:47:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 6: b |= ((uint64_t)in[5]) << 40;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:48:5: note: here
     case 5: b |= ((uint64_t)in[4]) << 32;
     ^~~~
src/libponyrt/ds/fun.c:48:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 5: b |= ((uint64_t)in[4]) << 32;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:49:5: note: here
     case 4: b |= ((uint64_t)in[3]) << 24;
     ^~~~
src/libponyrt/ds/fun.c:49:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 4: b |= ((uint64_t)in[3]) << 24;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:50:5: note: here
     case 3: b |= ((uint64_t)in[2]) << 16;
     ^~~~
src/libponyrt/ds/fun.c:50:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 3: b |= ((uint64_t)in[2]) << 16;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:51:5: note: here
     case 2: b |= ((uint64_t)in[1]) << 8;
     ^~~~
src/libponyrt/ds/fun.c:51:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 2: b |= ((uint64_t)in[1]) << 8;
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:52:5: note: here
     case 1: b |= ((uint64_t)in[0]);
     ^~~~
cc1: all warnings being treated as errors

[jemc]

@russel - Can you please let us know which gcc or clang version you are using to compile Pony?

[russel]

Apologies, I should have been explicit. I am using Fedora Rawhide GCC which is now 7.0.1.

[sylvanc]

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

It looks like gcc 7 has added a default fallthrough warning behaviour. Note that it isn't present in gcc 6:

https://gcc.gnu.org/onlinedocs/gcc-6.3.0/gcc/Warning-Options.html

The default level (3) accepts various regular expressions as "fallthrough indicators", along with a gcc specific attribute. I suggest we pick one that level 4 also recognises, and add it in fun.c (which appears to be the only place we use fallthrough). For example:

// -fallthrough

[russel]

@sylvanc @jemc Closing this was premature, I'm afraid. I pulled to bfb4b94 and then:

|> make
Makefile:224: WARNING: Unsupported LLVM version: 3.9.0
Makefile:225: Please use LLVM 3.7.1, 3.8.1, or 3.9.1
actor.c
messageq.c
fun.c
src/libponyrt/ds/fun.c: In function ‘siphash24’:
src/libponyrt/ds/fun.c:46:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 7: b |= ((uint64_t)in[6]) << 48; // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:47:5: note: here
     case 6: b |= ((uint64_t)in[5]) << 40; // -fallthrough
     ^~~~
src/libponyrt/ds/fun.c:47:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 6: b |= ((uint64_t)in[5]) << 40; // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:48:5: note: here
     case 5: b |= ((uint64_t)in[4]) << 32; // -fallthrough
     ^~~~
src/libponyrt/ds/fun.c:48:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 5: b |= ((uint64_t)in[4]) << 32; // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:49:5: note: here
     case 4: b |= ((uint64_t)in[3]) << 24; // -fallthrough
     ^~~~
src/libponyrt/ds/fun.c:49:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 4: b |= ((uint64_t)in[3]) << 24; // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:50:5: note: here
     case 3: b |= ((uint64_t)in[2]) << 16; // -fallthrough
     ^~~~
src/libponyrt/ds/fun.c:50:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 3: b |= ((uint64_t)in[2]) << 16; // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:51:5: note: here
     case 2: b |= ((uint64_t)in[1]) << 8;  // -fallthrough
     ^~~~
src/libponyrt/ds/fun.c:51:15: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 2: b |= ((uint64_t)in[1]) << 8;  // -fallthrough
             ~~^~~~~~~~~~~~~~~~~~~~~~~~~
src/libponyrt/ds/fun.c:52:5: note: here
     case 1: b |= ((uint64_t)in[0]);
     ^~~~
cc1: all warnings being treated as errors

|> gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/7/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap --enable-languages=c,c++,objc,obj-c++,fortran,ada,go,lto --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared --enable-threads=posix --enable-checking=release --enable-multilib --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-gnu-unique-object --enable-linker-build-id --with-gcc-major-version-only --with-linker-hash-style=gnu --enable-plugin --enable-initfini-array --with-isl --enable-libmpx --enable-offload-targets=nvptx-none --without-cuda-driver --enable-gnu-indirect-function --with-tune=generic --with-arch_32=i686 --build=x86_64-redhat-linux
Thread model: posix
gcc version 7.0.1 20170204 (Red Hat 7.0.1-0.6) (GCC) 

[jemc]

@russel if you could experiment with variations on the fallthrough comment, and with varying levels for this warning from -Wimplicit-fallthrough=4 to -Wimplicit-fallthrough=0, it would speed up the troubleshooting process a lot, as I don't currently have access to GCC 7.

You can change the compiler build flags in the Makefile here.

[russel]

@jemc Rather than just play, I will get advice from a person/people who will know the answer off the top of their head(s).

[jwakely]

Try removing the hyphen, or removing the space i.e. // fallthrough or //-fallthrough, but not // -fallthrough

// fallthrough matches the regex [ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )? fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?

//-fallthrough matches the regex -fallthrough (which is one of the standard Lint comments, which GCC recognises for compatibility with code that is already commented for Lint checking).

[russel]

@jemc I took the - out, and everything compiles. I'll proffer a pull request.

[russel]

@jwakely Thanks for the data, much appreciated.

[jemc]

Ah, I didn't realize that leading whitespace counted as part of the matching regular expression for -fallthrough. Just leaving out the - as you suggest seems like the best option.

@jwakely Thanks for the help, we appreciate it!

@russel Thanks, I'll look forward to the pull request.

[russel]

Pull request #1585 is currently running the tests.