[RFC] Enable `-Wunused` #36167

pull fanquake wants to merge 8 commits into bitcoin:master from fanquake:enable_wunused changing 160 files +607 −649

fanquake commented at 2:32 PM on September 4, 2026: member
We've had a handful of dead code removal PRs over the last month or two:
- #35679
- #35910
- #35914
- #35971
- #36009
- #36012
- #36103
- #36127
Also well as an instance of what was thought to be dead code, but it'd actually just been forgotten to be used: #36137.

It could be beneficial to get -Wunused & related flags enabled, to catch dead/unused code in CI.

A number of changes here need to go to subtrees:
- https://github.com/bitcoin-core/minisketch/pull/104
- https://github.com/bitcoin-core/libmultiprocess/pull/362
Enabling some of these flags may also help enforce other stuff, like inline constexpr usage (#35852). i.e:
```
 /home/runner/work/_temp/src/leveldb/db/dbformat.h:67:29: error: 'leveldb::kMaxSequenceNumber' defined but not used [-Werror=unused-const-variable=]
 67 | static const SequenceNumber kMaxSequenceNumber = ((0x1ull << 56) - 1);
 | ^~~~~~~~~~~~~~~~~~
```
DrahtBot commented at 2:32 PM on September 4, 2026: contributor


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



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36167
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36167.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36167#author-ralph


Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #bitcoin-core/gui/866 <img src="https://drahtbot.space/ack_count/bitcoin-core/gui/866.svg"> (Enhanced Traffic Graph Widget with Multi-timeframe Support and Data Persistence by rebroad)
- #36566 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36566.svg"> (private broadcast: wait synchronously for mempool acceptance by andrewtoth)
- #36509 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36509.svg"> (refactor: remove more dead code by fanquake)
- #36384 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36384.svg"> (rpc: Use self.Arg<_>(name) helper over manual and duplicate parsing by maflcko)
- #36326 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36326.svg"> (kernel: Use typed errors for fatal and flush error notifications by arejula27)
- #36268 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36268.svg"> (refactor: prune unused semi-colons by fanquake)
- #36264 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36264.svg"> (wallet rpc: fix stale argument metadata and help text by MrHodlX)
- #36183 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36183.svg"> (span: diagnose dangling views from MakeByteSpan/MakeUCharSpan by kevkevinpal)
- #36122 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36122.svg"> (BIP460: CISA for Taproot key path spends by fjahr)
- #36097 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36097.svg"> (mining: replace interrupt methods with cancellation arguments by xyzconstant)
- #35932 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35932.svg"> (ipc: make ipc::disconnectIncoming wait for in-progress calls to complete by ryanofsky)
- #35911 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35911.svg"> (Warn on and add missing [[noreturn]] by fanquake)
- #35906 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35906.svg"> (First steps towards a stateless, side-effect free validation library by purpleKarrot)
- #35760 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35760.svg"> (wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan by achow101)
- #35511 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35511.svg"> (RFC: consensus: Make CAmount a class by hodlinator)
- #35370 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35370.svg"> (rpc: add key-origin modes to PSBT processing RPCs by junbyjun1238)
- #35302 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35302.svg"> (Silent Payments: Sending (take 2) by Eunovo)
- #33324 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33324.svg"> (blocks: add resumable reobfuscation for existing block files by l0rinc)
- #33117 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33117.svg"> (Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications by D33r-Gee)
- #33112 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/33112.svg"> (wallet: relax external_signer flag constraints by Sjors)
- #32857 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/32857.svg"> (wallet: allow skipping script paths by Sjors)
- #32468 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/32468.svg"> (rpc: generateblock to allow multiple outputs by polespinasa)
- #32387 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/32387.svg"> (ipc: add windows support by ryanofsky)
- #31507 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/31507.svg"> (build: Use clang-cl to build on Windows natively by hebasto)
- #30343 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/30343.svg"> (wallet, logging: Replace WalletLogPrintf() with LogInfo() by ryanofsky)
- #29409 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/29409.svg"> (ipc: Expose Chain interface to IPC clients by ryanofsky)
- #19461 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19461.svg"> (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
- #19460 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/19460.svg"> (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
- #10102 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/10102.svg"> (Multiprocess bitcoin by ryanofsky)
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 added the label CI failed on Sep 4, 2026
DrahtBot commented at 3:54 PM on September 4, 2026: contributor


🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/33884346846/job/101060370925 LLM reason (✨ experimental): CI failed the subtree lint check because a subtree directory was modified without the required subtree merge (FAIL: subtree directory was touched without subtree merge).

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>
fanquake force-pushed on Sep 4, 2026
fanquake force-pushed on Sep 7, 2026
fanquake force-pushed on Sep 9, 2026
fanquake force-pushed on Sep 14, 2026
fanquake force-pushed on Sep 16, 2026

in src/policy/fees/mempool_estimator.h:38 in b8a063d2a5

  37 |  
  38 |  // Constants for mempool sanity checks.
  39 | -constexpr size_t MEMPOOL_HEALTH_WINDOW_BLOCKS = 6;
  40 | -constexpr double MEMPOOL_REPRESENTATION_THRESHOLD = 0.75;
  41 | +inline constexpr size_t MEMPOOL_HEALTH_WINDOW_BLOCKS = 6;
  42 | +inline constexpr double MEMPOOL_REPRESENTATION_THRESHOLD = 0.75;

maflcko commented at 10:46 AM on September 16, 2026:

Nice, but I think those should:

use the C++11 narrowing-check init `double ...{0.75};
Maybe be split up into a separate pull from the unused parameter warnings

Haven't tried this, but what about just a small pull to add:

try_append_cxx_flags("-Wunused-macros" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wunused-const-variable=2" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wunused-member-function" TARGET warn_interface SKIP_LINK)
try_append_cxx_flags("-Wunused-template" TARGET warn_interface SKIP_LINK)
etc...whatever...

And leave the -Wno-unused-parameter as-is for now?

Those should catch most of the issues mentioned in OP (except for the unused parameter ones)?

fanquake commented at 3:15 PM on September 16, 2026:

Split a portion out into #36275.

sipa referenced this in commit a12f5de1c9 on Sep 16, 2026
fanquake referenced this in commit 53a1914e00 on Sep 16, 2026
DrahtBot added the label Needs rebase on Sep 17, 2026
fanquake referenced this in commit 60bcf13edf on Sep 19, 2026
fanquake force-pushed on Sep 29, 2026
fanquake force-pushed on Sep 29, 2026
fanquake force-pushed on Sep 29, 2026
in src/rpc/blockchain.cpp:299 in aaa1307cc4 outdated
```
 295 | @@ -296,7 +296,7 @@ static RPCMethod getblockcount()
 296 |                      HelpExampleCli("getblockcount", "")
 297 |              + HelpExampleRpc("getblockcount", "")
 298 |                  },
 299 | -        [](const RPCMethod& self, const JSONRPCRequest& request) -> UniValue
 300 | +        [](const RPCMethod&, const JSONRPCRequest& request) -> UniValue
```
maflcko commented at 3:12 PM on September 29, 2026:

still not sure about enabling the unused-param one. I think we want to use self here more, not remove it. But using it would require touching this line again.

Maybe some of the other warnings can be enabled #36167 (review)?

fanquake commented at 4:54 PM on September 29, 2026:

I think we want to use self here more,

Are there any PRs currently doing this? I'll split more of this out.

maflcko commented at 8:06 PM on September 29, 2026:

Sure, see https://github.com/bitcoin/bitcoin/pull/36384
hebasto referenced this in commit 3b9768a07a on Sep 29, 2026
fanquake force-pushed on Sep 29, 2026
DrahtBot removed the label Needs rebase on Sep 29, 2026
fanquake force-pushed on Oct 1, 2026
hebasto referenced this in commit 69142eacd1 on Oct 2, 2026
DrahtBot added the label Needs rebase on Oct 2, 2026
fanquake force-pushed on Oct 2, 2026
DrahtBot removed the label Needs rebase on Oct 2, 2026
fanquake force-pushed on Oct 6, 2026
fanquake force-pushed on Oct 13, 2026

in CMakeLists.txt:487 in 45b90e51b5

 479 | @@ -480,9 +480,11 @@ else()
 480 |    try_append_cxx_flags("-Wthread-safety-pointer" TARGET warn_interface SKIP_LINK)
 481 |    try_append_cxx_flags("-Wloop-analysis" TARGET warn_interface SKIP_LINK)
 482 |    try_append_cxx_flags("-Wredundant-decls" TARGET warn_interface SKIP_LINK)
 483 | -  try_append_cxx_flags("-Wunused-member-function" TARGET warn_interface SKIP_LINK)
 484 | +  try_append_cxx_flags("-Wunused" TARGET warn_interface SKIP_LINK)
 485 |    try_append_cxx_flags("-Wunused-const-variable" TARGET warn_interface SKIP_LINK)
 486 | +  try_append_cxx_flags("-Wunused-member-function" TARGET warn_interface SKIP_LINK)
 487 |    try_append_cxx_flags("-Wunused-exception-parameter" TARGET warn_interface SKIP_LINK)
 488 | +  try_append_cxx_flags("-Wunused-template" TARGET warn_interface SKIP_LINK)

maflcko commented at 6:39 AM on October 13, 2026:

can you split this one out? Possibly also -Wunused-macros (if meaningful, etc)?

fanquake commented at 8:39 AM on October 13, 2026:

Put them into #36509.

sedited referenced this in commit d3c3871b59 on Oct 21, 2026
DrahtBot added the label Needs rebase on Oct 21, 2026

rpc: Use the first arg name in GetParamIndex

GetName does not support aliases at all. However, it should be fine for
GetParamIndex (used in Arg and MaybeArg) to support lookup by the first
name.

The test is modified to show the second legacy alias is ignored.

87cc1d2e71

rpc: doc: Use proper RPCArg::Default in verifychain 466422b3fd

refactor: rpc: Use self.Arg<_>(name) helper over manual isNull() ? fallback : getter()

The Arg() helper has many benefits:

* It does not require manual isNull checks
* It does not require hard-coding the fallback/default value a second time
* It does not require manually specifying the getter for the type. Specifying the type is enough.

So use it in this refactor, which does not change any behavior.

2bc576f98b

[[nomerge]] leveldb: unused changes 8f33c0480d
[[nomerge]] minisketch: unused changes 47a1a3e3c2
[[nomerge]] libmultiprocess changes 3a03e44204
refactor: fix -Wunused issues cfa503e68f
build: add -Wunused and friends ed751d5644
fanquake force-pushed on Oct 21, 2026
DrahtBot removed the label Needs rebase on Oct 21, 2026

Contributors

Labels

Linked (view graph)

#35679 fuzz: Remove unused `DeserializeFromFuzzingInput` params overload #35852 scripted-diff: Use inline const(expr) over static constexpr in headers #35910 refactor: Remove unused newFeeRate var in ReplacementChecks #35914 test, fuzz: Remove unused variables #35971 net_processing: remove unused code #36009 miniscript: remove unused context argument from ParseHexStr #36012 psbt: Remove unused `IsNull()` methods #36103 validation: remove unused code #36127 wallet: remove unused code #36137 validation: use unused SetTargetBlockHash #36275 build: enable `-Wunused-const-variable`#36383 util: remove unused exception parameter in `Popen::execute_process`#36509 refactor: remove more dead code

[RFC] Enable `-Wunused` #36167

External sites

Reviews

Conflicts