Page MenuHomePhabricator

Mark fall-through cases in our code clearly
ClosedPublic

Authored by freetrader on Jun 19 2017, 08:47.

Details

Summary

This should help to avoid unnecessary compiler warnings.
In practice it does not seem to work right despite -Wimplicit-fallthrough=3,
but it does not hurt to clearly mark the cases already.

Links:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/

Test Plan

make check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
mark_fallthrough_cases
Lint
Lint Passed
SeverityLocationCodeMessage
Auto-Fixsrc/hash.cpp:1CFMTCode style violation
Auto-Fixsrc/tinyformat.h:1CFMTCode style violation
Unit
No Test Coverage
Build Status
Buildable 335
Build 335: arc lint + arc unit

Event Timeline

Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptJun 19 2017, 08:47

I accepted on principle. Please format the files and then rebase this diff on top of the formatting before landing it.

This revision is now accepted and ready to land.Jun 19 2017, 10:39

Ok, I will re-format and change comment style to be consistent with the file.

I think you're right about not mixing C and C++ style comments unnecessarily, I used the C style comments since I copied from instructions which were supposedly working.
But checking the gcc info, C++ style should be accepted too, and maybe it will be treated better by clang-style.

Opened D227 and D228 for preliminary formatting as a basis for amending this one.

Updated on top of reformatted files

Changed to C++ comments instead of C

freetrader edited edge metadata.

I didn't test this yet. But you wrote

"In practice it does not seem to work right despite -Wimplicit-fallthrough=3"

Note that you used "FALLTHROUGH" but on this page you linked, https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/:

it says "FALLTHRU".

Might that be the reason explaining this?

This revision is now accepted and ready to land.Jun 21 2017, 10:29
src/qt/rpcconsole.cpp
272 ↗(On Diff #560)

Maybe after the closing }

In D224#3770, @awemany wrote:

I didn't test this yet. But you wrote

"In practice it does not seem to work right despite -Wimplicit-fallthrough=3"

Note that you used "FALLTHROUGH" but on this page you linked, https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/:

it says "FALLTHRU".

Might that be the reason explaining this?

The Redhat page seems a bit older and only gives one example syntax (which I tried), but the gcc page lists a bunch of regexp's of which I tried a lot, in combination with different -Wimplicit-fallthrough=n , on gcc 7.1.0 .
I was surprised I couldn't get rid of the warnings - I checked with make V=1 that they were invoked on the command line, but maybe some other options overrides.
I guess it must be something silly like that.

This revision was automatically updated to reflect the committed changes.