#36623 build: enable `-Wunreachable-code-aggressive`

ReviewabilityReviewsAgreementSize
#36623 fanquake build: enable `-Wunreachable-code-aggressive`CI failing0Crickets11
  • Catch dead break and return statements during compilation
  • Prevent unreachable control-flow statements from entering the codebase
  • CI is failing, so check if the failure is caused by this PR before deep review
  • No review verdicts recorded
  • No substantive comments on the overall approach yet
  • Suggested removing a break and braces in script evaluation instead of suppressing the warning (hodlinator)
  • Considered removing additional unreachable code, leaving choice to reviewer preference (willcl-ark)
  • 11 lines added or modified outside tests
  • 0 lines added or modified in tests
  • 6 lines removed in total
  • 5 files, 3 commits
Build and CI#49 of 56
P4 · cleanup
  • P4 because compiler warning flag tweaks provide routine code hygiene
  • Offers minimal leverage compared to broader platform support or build reliability
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Replaces compiler flag `-Wunreachable-code` with `-Wunreachable-code-aggressive` in CMakeLists.txt. Fixes dead break and return statements triggered by the aggressive check in Qt and utility code, silences an unreachable break warning in the script interpreter using Clang pragmas, and includes a placeholder commit for Minisketch.

Problem: The standard `-Wunreachable-code` flag does not diagnose unreachable `break` or `return` statements, allowing dead control-flow statements to slip into the codebase.

Evidence of importance:

  • Commit 537d2882f4 states: 'Enables -Wunreachable-code-break and -Wunreachable-code-return.'

Scope: Touches vendored subtree code in `src/minisketch` with a `[nomerge]` commit, and touches consensus code in `src/script/interpreter.cpp` with `#pragma clang diagnostic ignored`.

Risk of a mistake: medium (build, consensus). A mistake when altering control flow or suppressing warnings in the script interpreter could inadvertently change consensus evaluation.

Changed since the previous assessment: Dropped the vendored LevelDB commit and its dependency on leveldb-subtree PR 66, leaving only the Minisketch nomerge commit for external subtrees.

Reviewability

Ready · CI failing

CI is red; check whether the failure is the PR's before a deep review.

Waiting on (from the thread read):

  • author: consider hodlinator's suggestion to remove break and braces in EvalScript 2026-10-31

Agreement

Crickets

no substantive comment either way

Thread notes: Added hodlinator's suggestion on formatting the OP_RETURN case in EvalScript without braces and break.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
willcl-ark (member)suggestion, scopeConsider removing this code as wellanswered · author leaves it up to reviewer preferenceansweredyes2026-10-23: 'It does appear that this is likely removable too.'
Status from: 2026-10-30: fanquake: 'Happy to do what reviewers prefer here.'
hodlinator (member)suggestion, approachRemove the break together with the braces around OP_RETURN in EvalScriptopenansweredno2026-10-31: 'the least worst solution to me seems to be removing the break together with the braces'

Participants:

  • willcl-ark (member, neutral, 1 statements 2026-10-23 to 2026-10-23): suggested additional code in script interpreter may be removable
  • hodlinator (member, neutral, 1 statements 2026-10-31 to 2026-10-31): suggested removing the break and braces around OP_RETURN

Files

0 lines under test/bench/ci.

  • src/script/interpreter.cpp +7/-0
  • src/qt/rpcconsole.cpp +3/-3
  • CMakeLists.txt +1/-1
  • src/minisketch/src/minisketch.cpp +0/-1
  • src/util/tokenpipe.cpp +0/-1

Card

The short text the ranking pass compares across PRs.

Enables the compiler warning flag -Wunreachable-code-aggressive to catch unreachable break and return statements. Cleans up dead code across Qt and utility helpers, silences a diagnostic in the script interpreter via pragmas, and carries a placeholder commit for vendored Minisketch code. It catches dead control-flow code earlier during compilation.

Data

dossier JSON · extract JSON · model ledger, generated 2026-11-01T01:17, confidence high, input hash 6b13bfbd2b325271
thread read through 2026-10-31 (3 statements), head 537d2882f4
labels: Build system, CI failed · draft

Categories

Build and CI

P4 · cleanup

Enabling -Wunreachable-code-aggressive diagnoses dead break and return statements during compilation. While useful for code hygiene, compiler diagnostic flag tweaks have no special claim on review attention compared to platform support or CI reliability.

Membership: Carries the Build system label and modifies CMakeLists.txt to enable the -Wunreachable-code-aggressive compiler flag.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0