[RFC] Enable `-Wunused` #36167

pull fanquake wants to merge 10 commits into bitcoin:master from fanquake:enable_wunused changing 161 files +613 −653

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.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36167.



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:
- #36383 (util: remove unused exception parameter in Popen::execute_process by fanquake)
- #36326 (kernel: Use typed errors for fatal and flush error notifications by arejula27)
- #36268 (refactor: prune unused semi-colons by fanquake)
- #36122 (BIP460: CISA for Taproot key path spends by fjahr)
- #36097 (mining: replace interrupt methods with cancellation arguments by xyzconstant)
- #36074 (scripted-diff: [test] Add util/check.h includes for assertions by maflcko)
- #35911 (Warn on and add missing [[noreturn]] by fanquake)
- #35906 (First steps towards a stateless, side-effect free validation library by purpleKarrot)
- #35760 (wallet: make corrupted transaction records fail wallet loading instead of forcing a rescan by achow101)
- #35744 (coins: prevent DB resize from invalidating cursors by l0rinc)
- #35474 (node: move index ownership to NodeContext by w0xlt)
- #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
- #33324 (blocks: add resumable reobfuscation for existing block files by l0rinc)
- #33117 (Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications by D33r-Gee)
- #29409 (multiprocess: Add capnp wrapper for Chain interface 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. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/33884346846/job/101060370925</sub> <sub>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).</sub>

<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

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.

c9e76f61c0

rpc: doc: Use proper RPCArg::Default in verifychain 51c03192df

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.

5c40159aef

util: remove unused exception parameter in Popen::execute_process 046925be5b
build: enable -Wunused-exception-parameter 075dd40954
[[nomerge]] leveldb: unused changes 64c9211f84
[[nomerge]] minisketch: unused changes 1cdb3b87b4
[[nomerge]] libmultiprocess changes 3779e94d8f
refactor: fix -Wunused issues 845aa9bfe6
build: add -Wunused and friends 5886b59f6c
fanquake force-pushed on Oct 1, 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`

[RFC] Enable `-Wunused` #36167

Code Coverage & Benchmarks

Reviews

Conflicts