Page MenuHomePhabricator

Merge #10489: build: silence gcc7's implicit fallthrough warning
ClosedPublic

Authored by jasonbcox on Tue, May 7, 02:52.

Details

Summary

cf390df build: silence gcc7's implicit fallthrough warning (Cory Fields)

Tree-SHA512: 330a4258161529c6e7af34ba3dc9703c24d897d7fff317c078441405c4546c7236e260603181001954b214d4640ad09ed3d34d3b518be991cb1712a02bcaa1d9

Backport of Core PR10489
https://github.com/bitcoin/bitcoin/pull/10489/files
I noticed these fallthrough warnings appearing repeatedly due to tinyformat as the original PR describes.
Although I don't 100% agree that it is good to always silence this warning, it is proving to add
cognitive load when digging through real build error messages. I'll leave this diff up for discussion.

Test Plan
make # no longer vomits warnings related to fallthrough
ninja # does not show fallthrough warnings
cmake -GNinja -DEXTRA_WARNINGS=1 ..
ninja # vomits fallthrough warnings as expected

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox created this revision.Tue, May 7, 02:52
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, May 7, 02:52
Fabien requested changes to this revision.Tue, May 7, 07:04

Can you also add it to src/CMakeLists.txt ?

This revision now requires changes to proceed.Tue, May 7, 07:04
jasonbcox edited the summary of this revision. (Show Details)Thu, May 9, 00:15
jasonbcox updated this revision to Diff 8537.Thu, May 9, 00:25

Rebase + added to CMake build

jasonbcox edited the test plan for this revision. (Show Details)Thu, May 9, 00:27
Fabien requested changes to this revision.Thu, May 9, 06:35
Fabien added inline comments.
src/CMakeLists.txt
92 ↗(On Diff #8537)

We want to remove it because it is part of Wextra since gcc7. No need to enable it first.

This revision now requires changes to proceed.Thu, May 9, 06:35
jasonbcox updated this revision to Diff 8553.Thu, May 9, 16:07

Removed enabling implicit-fallthrough as the default

Fabien accepted this revision.Thu, May 9, 16:13
This revision is now accepted and ready to land.Thu, May 9, 16:13
This revision was automatically updated to reflect the committed changes.