Page MenuHomePhabricator

[build] enforce exhaustive switch statements in BUILD_WERROR config
ClosedPublic

Authored by majcosta on May 17 2020, 01:47.

Details

Summary

D6079 introduced a function that switches over an enum class parameter
and returns a std::string, which raises a warning from the compiler that
"control reaches end of non-void function". since this should never
happen, an assert(false) after the switch silences that warning.

to really bulletproof it though, defaultless switch statements over an enum
value that do not have a specific case for each should raise an error and not a warning

edit: also promoted -Wreturn-type to an error, h/t @deadalnix

ref: https://abseil.io/tips/147

Test Plan

add another value to PSBTRole enum class in src/psbt.h without changing the
definition of std::string PSBTRoleName(PSBTRole role);

cmake .. -GNinja -WENABLE_WERROR:BOOL=True
ninja

notice that this raises an error:

../src/psbt.cpp:282:13: error: enumeration value 'TESTER' not handled in switch [-Werror,-Wswitch]
switch (role) {

without the WENABLE_WERROR flag set, it'll only emit a warning.

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

majcosta created this revision.May 17 2020, 01:47
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 17 2020, 01:48
majcosta requested review of this revision.May 17 2020, 01:48
majcosta edited the summary of this revision. (Show Details)May 17 2020, 02:25
deadalnix requested changes to this revision.May 17 2020, 02:25
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/CMakeLists.txt
182 ↗(On Diff #20176)

Please do not make warning errors, because they are different accross compilers.

This revision now requires changes to proceed.May 17 2020, 02:25
majcosta updated this revision to Diff 20177.May 17 2020, 13:30

addressed comment

deadalnix accepted this revision.May 17 2020, 14:52
This revision is now accepted and ready to land.May 17 2020, 14:52
majcosta updated this revision to Diff 20178.May 17 2020, 15:23

since we can't promote warnings to errors in the default build due to toolchains being all over the place,we should probably move those to our own CIs ENABLE_WERROR config so we get the benefit without having to know and support every compiler warning flag ever released

majcosta retitled this revision from [build] enforce exhaustive switch statements to [build] enforce exhaustive switch statements in BUILD_WERROR config.May 17 2020, 15:26
majcosta edited the summary of this revision. (Show Details)
majcosta edited the test plan for this revision. (Show Details)
majcosta edited the summary of this revision. (Show Details)May 17 2020, 15:27