Page MenuHomePhabricator

[CMAKE] Make warnings the same with cmake and autotools

Authored by Fabien on Jul 31 2019, 14:31.


Group Reviewers
Restricted Project

This avoids getting different warnings when running cmake or autotools.
It moves secp256k1 flags to the appropriated CMakeLists.txt, avoids
flags duplication and enable -Wthread-safety-analysis with cmake.

The leveldb flags have not been updated and cmake still removes some
more warnings than autotools.

Test Plan
make check
ninja check bitcoin-bench

Check there is no difference in the compiler warnings.

Diff Detail

rABC Bitcoin ABC
Lint OK
No Unit Test Coverage
Build Status
Buildable 7020
Build 12087: Bitcoin ABC Buildbot (legacy)
Build 12086: arc lint + arc unit

Event Timeline

Fabien created this revision.Jul 31 2019, 14:31
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 31 2019, 14:31
deadalnix requested changes to this revision.Aug 1 2019, 06:21

I don't see any point in removing warning for which the codebase is green.

This revision now requires changes to proceed.Aug 1 2019, 06:21
Fabien requested review of this revision.Aug 1 2019, 09:51

There is no real warning removal, but some are moved and so getting narrow-scoped:

  • -Wshadow is no longer duplicated in secp256k1, it remains set at the top level;
  • -Wnested-externs and -Wstrict-prototypes are C-only flags and only apply to secp256k1 with autotools, so I moved them;
  • The main "loss" here can be -Wcast-align, which is not green in the codebase and now applies to secp256k1 only (making it effectively green, just like autotools).
  • There is one additional warning, which is -Wthread-safety-analysis (greened by other diffs), which is already enabled with autotools.

Let me know if you think that moving the warnings to secp256k1 is undesirable, and I'll only keep the -Wthread-safety-analysis change.

deadalnix requested changes to this revision.EditedAug 5 2019, 05:45

If the scope is narrowed, then the warning was removed from the rest of the codebase. This is a net negative.

This revision now requires changes to proceed.Aug 5 2019, 05:45
Fabien abandoned this revision.Aug 6 2019, 09:20

Superseeded by D3798 and D3799