build: Add -Werror=unused... compile flags #21458

pull hebasto wants to merge 4 commits into bitcoin:master from hebasto:210317-error changing 1 files +8 −4

hebasto commented at 1:34 PM on March 17, 2021: member
With C++17 [[maybe_unused]] attribute it is easy to keep emerged cases in the new code under control.

This PR does not touch -Wno-unused-parameter as it requires too much of code churning.

Compiler specific notes:
1. clang:
- -Wunused-macros not added because it requires a non-trivial checking in the configure script, and it blames only https://github.com/bitcoin/bitcoin/blob/a9d1b40d53ec417eefbe767aa66701ef8e1801d5/src/util/system.cpp#L34
- -Wunused-template not added because some of our templates are buried into macros
- -Wused-but-marked-unused produces tones of warnings in boost tests
1. gcc
- -Werror=unused-const-variable with level 2 seems not suitable for C++ code
fanquake added the label Build system on Mar 17, 2021
DrahtBot cross-referenced this on Mar 17, 2021 from issue build: Add convenient BITCOIN_TRY_ADD_COMPILE_FLAG macro by hebasto
DrahtBot commented at 8:50 PM on March 17, 2021: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #23191 (build: Disable -Wbraced-scalar-init, which is incompatible with -Wc++11-narrowing by MarcoFalke)
- #23149 (build: make --enable-werror just -Werror by fanquake)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot cross-referenced this on Mar 17, 2021 from issue build: Add -Werror=implicit-fallthrough compile flag by hebasto
DrahtBot cross-referenced this on Mar 18, 2021 from issue Fix Windows build with --enable-werror by hebasto
practicalswift commented at 9:22 PM on March 19, 2021: contributor

Concept ACK, but why the removal of -Werror=unused-variable? :)

Could you describe some tests cases that could be used to verify that the patch in this PR makes the build fail where we expect it to fail?
DrahtBot cross-referenced this on Apr 6, 2021 from issue build: remove -Wdeprecated-register from NOWARN flags by fanquake
DrahtBot added the label Needs rebase on Apr 6, 2021
build: Promote -Wunused-variable to -Wunused 86bb9d8e9f

build: Drop -Wno-unused-local-typedef

-Wunused-local-typedef(s) is covered by -Wunused in both gcc and clang.
No new warnings fire when compiling.

33a3554cbb

build: Add clang specific -Werror=unused-... compile flags

Added flags are disabled by default.
No new warnings fire when compiling.

6e9ab1045a

build: Add gcc specific -Werror=unused-const-variable=1 compile flag

Added flag is disabled by default for C++.
No new warnings fire when compiling.

424683d349

hebasto force-pushed on Aug 24, 2021
hebasto commented at 6:15 PM on August 24, 2021: member

Rebased 22dd4fbf5d123976b2b23860188115cbea036cbd -> 424683d349dd9a748b324b74a58e034abd76a33e (pr21458.01 -> pr21458.02).
DrahtBot removed the label Needs rebase on Aug 24, 2021
DrahtBot cross-referenced this on Oct 1, 2021 from issue build: make --enable-werror just -Werror by fanquake
DrahtBot cross-referenced this on Oct 6, 2021 from issue build: Disable -Wbraced-scalar-init, which is incompatible with -Wc++11-narrowing by MarcoFalke
DrahtBot added the label Needs rebase on Oct 13, 2021
DrahtBot commented at 5:11 AM on October 13, 2021: contributor



🐙 This pull request conflicts with the target branch and needs rebase.

<sub>Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".</sub>
fanquake cross-referenced this on Oct 13, 2021 from issue build: remove redundant warning flags by fanquake

in configure.ac:451 in 424683d349

 447 | @@ -445,8 +448,10 @@ if test "x$CXXFLAGS_overridden" = "xno"; then
 448 |    AX_CHECK_COMPILE_FLAG([-Wthread-safety],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]])
 449 |    AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]])
 450 |    AX_CHECK_COMPILE_FLAG([-Wredundant-decls],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"],,[[$CXXFLAG_WERROR]])
 451 | -  AX_CHECK_COMPILE_FLAG([-Wunused-variable],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-variable"],,[[$CXXFLAG_WERROR]])
 452 | -  AX_CHECK_COMPILE_FLAG([-Wunused-member-function],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-member-function"],,[[$CXXFLAG_WERROR]])
 453 | +  AX_CHECK_COMPILE_FLAG([-Wunused], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused"], [], [$CXXFLAG_WERROR])

fanquake commented at 5:41 AM on October 13, 2021:

In 86bb9d8e9f5e0991bd443c7242b9b8a78704d4eb: -Wunused is already part of -Wall (via -Wmost) for Clang, and is implied by -Wall for GCC, so this should be redundant.

fanquake commented at 5:48 AM on October 13, 2021: member

~0. I'm not super convinced about the addition of (post rebase, and without -Wunused) two, specific -Wunused-* flags. See also #23269 where I've cherry-picked 33a3554cbb30657b9af34802dd610b286aa71a56.
fanquake referenced this in commit a845f1ccc6 on Oct 14, 2021
sidhujag referenced this in commit aad5627fc4 on Oct 14, 2021
fanquake commented at 12:46 PM on November 25, 2021: member

This needs a rebase, and comments addressed, or can probably be closed.
hebasto closed this on Nov 28, 2021
hebasto deleted the branch on Nov 28, 2021
bitcoin locked this on Nov 28, 2022

Contributors

Labels

Build system Needs rebase

Linked (view graph)

#23269 build: remove redundant warning flags

build: Add -Werror=unused... compile flags #21458

Conflicts