util: Add and use AssertUnreachable #36087

pull maflcko wants to merge 7 commits into bitcoin:master from maflcko:2608-unreachable changing 312 files +790 −546

maflcko commented at 5:24 AM on August 26, 2026: member
The current code uses assertions with falsy literal values in a few places. This is perfectly fine, but this comes with a bit of confusion sometimes:
- assert(false) may cause confusion when it is unclear if NDEBUG can disable it. (The intention is to disallow NDEBUG compilation)
- Assert(false) hides the noreturn attribute behind a function call, which may cause GCC return-type warnings. (Clang seems to understand it) E.g. https://godbolt.org/z/e7bWsPn51
- When C++23 will be allowed, devs may prefer std::unreachable. However, this is unsafe and will invoke UB.
- There is confusion why a CHECK_NONFATAL macro exists, but no fatal equivalent.
Fix all those issues by:
- Adding the util/check.h include for all assertions to import the NDEBUG compile error. (scripted-diff + clang-format)
- Adding a new AssertUnreachable, which directly calls the noreturn assertion_fail helper.
- Adding an assert_falsy linter to enforce all code to use this macro.
- Adding a linter to forbid std::unreachable.

scripted-diff: Add util/check.h includes for assertions

The project has a compile error when compiled without assertions in
util/check.h. Thus, util/check.h should be included for all assertions.

So do that with a scripted-diff for Assert and assert, and remove the
cassert include, which is exported from util/check.h.

-BEGIN VERIFY SCRIPT-

 paths=(
   'src' \
   ':(exclude)src/bench/nanobench.h' \
   ':(exclude)src/tinyformat.h' \
   ':(exclude)src/univalue' \
   ':(exclude)src/crc32c' \
   ':(exclude)src/crypto/ctaes' \
   ':(exclude)src/ipc/libmultiprocess' \
   ':(exclude)src/leveldb' \
   ':(exclude)src/minisketch' \
   ':(exclude)src/secp256k1' \
 )

 # Add the util/check.h includes
 for f in $(git grep -l --extended-regexp "\<(a|A)ssert\(" -- "${paths[@]}"); do
   if ! grep --quiet "util/check.h" "$f"; then
     line=$(grep --line-number --max-count=1 '^#include' "$f" | cut --delimiter=: --fields=1)
     sed --in-place "${line}i#include <util/check.h>" "$f"
   fi
 done

 # Remove cassert includes
 for f in $(git grep -l '<cassert>' -- "${paths[@]}"); do
   sed --in-place '/^#include <cassert>$/d' "$f"
 done

-END VERIFY SCRIPT-

fae5e7437a

refactor: Add missing #include external_signer.h to external_signer_scriptpubkeyman.h
This include is required and this change is also required for the next commit.
fab2b024f7

refactor: [test] Sort includes

This can be reproduced by checking out the previous commit and running:
git show -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v

fae4f21d0e

DrahtBot added the label Utils/log/libs on Aug 26, 2026
DrahtBot commented at 5:24 AM on August 26, 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/36087.



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/954 (Add dialog to select change output when bumping fee by pablomartin4btc)
- #bitcoin-core/gui/945 (qt: Fix sign message address book filtering by Bushstar)
- #bitcoin-core/gui/762 (Update about logo icon (colour) to denote the chain type of the QT instance in About/ Help Message Window/ Dialog by pablomartin4btc)
- #36091 (test: Add debug output to common tested types by rustaceanrob)
- #36088 (util: Set Univalue to null after read failure by maflcko)
- #36074 (scripted-diff: [test] replace assert with Assert by maflcko)
- #36068 (fuzz: reuse one fuzzed wallet across inputs by brunoerg)
- #36015 (txorphanage: bound orphan memory by storing transactions serialized by brunoerg)
- #35998 (wallet: Handle or explicitly ignore WalletBatch write failures by achow101)
- #35916 (fuzz: improve ipc fuzz coverage by enirox001)
- #35911 (Warn on and add missing [[noreturn]] by fanquake)
- #35887 (ipc: use std::optional for checkSpawned(), add tests and rename arg -ipcfd to -ipcchild by ViniciusCestarii)
- #35752 (wallet: make encryption state updates atomic by l0rinc)
- #35731 (Indexes: Harden the flush-error notification invariant by arejula27)
- #35714 (validation: stop writes after flush failure by l0rinc)
- #35713 (Remove boost as a unit test runner by rustaceanrob)
- #35511 (RFC: consensus: Make CAmount a class by hodlinator)
- #35461 (util: Clarify the assertion message in Assert, Assume and CHECK_NONFATAL by optout21)
- #35429 (wallet: avoid global access in external signer SPKM by w0xlt)
- #35351 (net: Disallow invalid HeadersSyncState due to lagging clock by hodlinator)
- #35301 (Silent Payments: Implement bip352 (take 2) by Eunovo)
- #35003 (validation: improve block data I/O error handling in P2P paths by furszy)
- #34861 (wallet: Add importdescriptors interface by polespinasa)
- #34778 (logging: rewrite macros to enforce restrictions at compile-time, improve efficiency and usability by ryanofsky)
- #34681 (wallet: move rescan logic into ChainScanner and wallet/scan by Eunovo)
- #34603 (wallet: Fix detection of symlinks on Windows by achow101)
- #34566 (feature: Use different datadirs for different signets by ekzyis)
- #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
- #34083 (Add initial vectorized chacha20 implementation for 2-3x speedup by theuni)
- #32729 (test,script: add sigop helpers (without consensus migration) by l0rinc)
- #32387 (ipc: add windows support by ryanofsky)
- #31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
- #31252 (rpc: print P2WSH and P2SH redem Script in getrawtransaction and getblock by polespinasa)
- #30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)
- #29409 (multiprocess: Add capnp wrapper for Chain interface by ryanofsky)
- #29278 (Wallet: Add maxfeerate wallet startup option by ismaelsadeeq)
- #29256 (log, refactor: Allow log macros to accept context arguments by ryanofsky)
- #27260 (Enhanced error messages for invalid network prefix during address parsing. by portlandhodl)
- #10102 (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.

util: Add AssertUnreachable

Also, in the tests convert a falsy literal to a "runtime" zero value.

fa8bc49290

lint: Add assert_falsy linter 76bf7f82ce

scripted-diff: Replace falsy assertions with AssertUnreachable

Replace assertions with falsy literal values by the explicit unreachable-code assertion macro.

-BEGIN VERIFY SCRIPT-

 paths=(
   'src'
   ':(exclude)src/tinyformat.h'
   ':(exclude)src/univalue/'
   ':(exclude)src/crc32c'
   ':(exclude)src/crypto/ctaes'
   ':(exclude)src/ipc/libmultiprocess'
   ':(exclude)src/leveldb'
   ':(exclude)src/minisketch'
   ':(exclude)src/secp256k1'
 )

 sed --in-place --regexp-extended 's/\<(a|A)ssert\((0|false)\)/AssertUnreachable()/g' $( \
   git grep -l --extended-regexp '\<(a|A)ssert\((0|false)\)' -- "${paths[@]}" \
 )

 sed --in-place --regexp-extended 's/assert\((0|false) && "([^"]*)"\);/AssertUnreachable(); \/\/ \2/g' $( \
   git grep -l --extended-regexp '\<assert\((0|false) &&' -- "${paths[@]}" \
 )

-END VERIFY SCRIPT-

cffcf07869

lint: Add std_unreachable linter acd644e130
maflcko force-pushed on Aug 26, 2026
maflcko marked this as a draft on Aug 26, 2026
DrahtBot added the label CI failed on Aug 26, 2026
DrahtBot commented at 5:45 AM on August 26, 2026: contributor


🚧 At least one of the CI tasks failed. <sub>Task lint: https://github.com/bitcoin/bitcoin/actions/runs/32933915544/job/98071384264</sub> <sub>LLM reason (✨ experimental): CI failed because the assert_falsy lint check detected an assert(0 && ...) (in src/tinyformat.h) that must be replaced with AssertUnreachable().</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>
DrahtBot added the label Needs rebase on Aug 26, 2026
DrahtBot commented at 3:52 PM on August 26, 2026: contributor



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

Contributors

maflcko

DrahtBot

Labels

Utils/log/libs Needs rebase CI failed

util: Add and use AssertUnreachable #36087

Code Coverage & Benchmarks

Reviews

Conflicts