ci, iwyu: Fix warnings in `src/util` and treat them as errors by hebasto · Pull Request #34448 · bitcoin/bitcoin

hebasto · 2026-01-29T15:38:39Z

This PR continues the ongoing effort to enforce IWYU warnings.

See Developer Notes.

DrahtBot · 2026-01-29T15:39:06Z

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

Reviews

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34641 (node: scale default -dbcache with system RAM by l0rinc)
#34639 (iwyu: Document or remove some pragma: export and other improvements by hebasto)
#34598 (bench: use deterministic HexStr payload by l0rinc)
#34435 (refactor: use _MiB/_GiB consistently for byte conversions by l0rinc)
#32554 (bench: replace embedded raw block with configurable block generator by l0rinc)

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.

fanquake · 2026-01-29T15:42:25Z

In file included from /home/admin/actions-runner/_work/_temp/src/hash.h:16:
/home/admin/actions-runner/_work/_temp/src/uint256.h:150:17: error: no member named 'HexDigit' in the global namespace
  150 |         *p1 = ::HexDigit(str[--digits]);
      |                 ^~~~~~~~
/home/admin/actions-runner/_work/_temp/src/uint256.h:152:38: error: no member named 'HexDigit' in the global namespace
  152 |             *p1 |= ((unsigned char)::HexDigit(str[--digits]) << 4);
      |                                      ^~~~~~~~
2 errors generated.

DrahtBot · 2026-01-29T15:48:42Z

🚧 At least one of the CI tasks failed.
_{Task Valgrind, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/21484505843/job/61889746047}
_{LLM reason (✨ experimental): Compilation failed due to missing #include causing std::vector to be undefined in string.h and related code.}

Hints

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.

fanquake · 2026-01-29T16:33:07Z

Seems pretty odd that IWYU can be "passing" here, but nothing actually compiles.

contrib/devtools/iwyu/bitcoin.core.imp

src/util/batchpriority.cpp

…ETE_FLAG` 516be10 wallet: Rename `RecordType::DELETE` to `RecordType::DELETE_FLAG` (Hennadii Stepanov) Pull request description: On Windows, the `winnt.h` header defines `DELETE` as a macro for a "Standard Access Right" bitmask (0x00010000L). This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before the `RecordType` enum definition, the preprocessor expands `DELETE` into a numeric literal, causing syntax errors. Rename the enumerator to `DELETE_FLAG` to remove this fragility and avoid the collision entirely. Split from #34448. ACKs for top commit: maflcko: re-lgtm ACK 516be10 achow101: ACK 516be10 Tree-SHA512: eba054b395e18c07efb2901b28f542b042b62d85e1a798eeff35f8431530cb667fa791c47c4125cecdb689213b458ba396715495415e9b83bb322509a9376222

hebasto · 2026-01-30T21:27:44Z

Rebased on top of merged #34454.

hebasto · 2026-01-31T09:15:16Z

Rebased on top of merged #34460.

hebasto · 2026-02-10T15:55:30Z

Rebased.

hebasto · 2026-02-10T16:29:44Z

unrelated, but the clang-format in the ci container seems broken? It removes the newlines. E.g. https://github.com/bitcoin/bitcoin/actions/runs/21830259022/job/62995439943#step:11:1639

Looks like fix_includes.py strips empty lines and clang-format-diff.py fails to restore them.

hebasto · 2026-02-10T17:24:57Z

unrelated, but the clang-format in the ci container seems broken? It removes the newlines. E.g. https://github.com/bitcoin/bitcoin/actions/runs/21830259022/job/62995439943#step:11:1639

Looks like fix_includes.py strips empty lines and clang-format-diff.py fails to restore them.

Fixed in #34551.

hebasto · 2026-02-10T18:09:52Z

The only conflicting PR has been merged, so now seems like a good time to get this reviewed.

Friendly ping @sedited @maflcko @willcl-ark :)

src/util/time.cpp

src/util/threadinterrupt.h

f8d2f30 ci: Extend diff context for clang-format (Hennadii Stepanov) Pull request description: This PR ensures `clang-format` can properly restore empty lines between header groups that were previously stripped by `fix_includes.py`. Addresses [this](#34448 (comment)) comment. ACKs for top commit: maflcko: lgtm ACK f8d2f30 l0rinc: untested code review ACK f8d2f30 sedited: ACK f8d2f30 Tree-SHA512: 97e5450fc15e415134aa1e74ee415f947173978d999afbc0f204e1cdb2f68309b763c8a2ad819c4597147fc15540600aa8c99477db64e961ad428d8f45f3d384

hebasto · 2026-02-13T14:13:36Z

Rebased.

DrahtBot · 2026-02-13T15:17:05Z

🚧 At least one of the CI tasks failed.
_{Task Windows-cross to x86_64, ucrt: https://github.com/bitcoin/bitcoin/actions/runs/21989888993/job/63533810608}
_{LLM reason (✨ experimental): Test failure: std::future::wait_for is called with a plain long int instead of a std::chrono::duration, causing a type-mismatch in threadpool_tests.cpp.}

Hints

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.

src/util/fs.h

On Windows, the `winerror.h` header defines `WAIT_TIMEOUT` as a macro. This introduces a fragile dependency on header inclusion order: if Windows headers happen to be included before using `WAIT_TIMEOUT`, the preprocessor expands it into a numeric literal, causing syntax errors. Rename the variable to `TEST_WAIT_TIMEOUT` to remove this fragility and avoid the collision entirely.

fanquake · 2026-02-18T19:01:01Z

src/logging.h

@@ -8,6 +8,7 @@

 #include <crypto/siphash.h>
 #include <logging/categories.h> // IWYU pragma: export


If this PR is going to fix util, then I think pretty much any (existing) pramga use, outside of the config header, should be getting comments (especially given the prior added one was another IWYU bug). Pragmas should be used sparingly, and explained when so.

I agree that all pragmas should eventually be documented. However, addressing the existing ones here would significantly expand the scope of this PR and likely introduce more merge conflicts.

Since these older pragmas might need to be removed entirely rather than just documented, it would be best to loop in the original authors or reviewers to make that call. I think it makes the most sense to tackle this cleanup in follow-up PRs.

However, addressing the existing ones here would significantly expand the scope of this PR

Looking at the current state of this PR, there are 4 instances of // IWYU pragma: export in src/util/. Documenting or removing them, seems in scope for a PR that is fixing all issues in src/util/. If not done now, we'll just have to come back again later and fix them anyways (and still have merge conflicts anyways, if there are any).

Since these older pragmas might need to be removed entirely rather than just documented,
it would be best to loop in the original authors or reviewers to make that call.

Isn't the best case if you can just delete the pragma, and fix the includes? Otherwise I'm not sure what call they would need to make. Either the pragma is removed, and includes fixed, there is a IWYU bug (which can be documented), or a rationale is written for keeping the pragma (even if it isn't needed)?

hebasto · 2026-02-20T17:54:55Z

Drafted. Please review #34639 first.

hebasto added Refactoring Utils/log/libs labels Jan 29, 2026

hebasto marked this pull request as draft January 29, 2026 15:41

DrahtBot added the CI failed label Jan 29, 2026

hebasto force-pushed the 260129-iwyu-util branch 4 times, most recently from c51291c to 154a854 Compare January 29, 2026 22:34

hebasto mentioned this pull request Jan 29, 2026

wallet: Rename RecordType::DELETE to RecordType::DELETE_FLAG #34454

Merged

hebasto force-pushed the 260129-iwyu-util branch from 154a854 to 2298908 Compare January 29, 2026 23:11

DrahtBot removed the CI failed label Jan 30, 2026

This was referenced Jan 30, 2026

kernel: use structured logging and simplify logging interface #34374

Draft

ci, iwyu: Fix warnings in src/zmq and treat them as errors #34338

Merged

fanquake reviewed Jan 30, 2026

View reviewed changes

contrib/devtools/iwyu/bitcoin.core.imp Show resolved Hide resolved

fanquake reviewed Jan 30, 2026

View reviewed changes

contrib/devtools/iwyu/bitcoin.core.imp Outdated Show resolved Hide resolved

maflcko reviewed Jan 30, 2026

View reviewed changes

src/util/batchpriority.cpp Outdated Show resolved Hide resolved

hebasto mentioned this pull request Jan 30, 2026

Add missing <bits/types/struct_sched_param.h> mapping to <sched.h> include-what-you-use/include-what-you-use#1918

Merged

hebasto force-pushed the 260129-iwyu-util branch from 2298908 to deca8eb Compare January 30, 2026 15:45

DrahtBot mentioned this pull request Jan 30, 2026

refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation #33878

Merged

DrahtBot added CI failed and removed CI failed labels Jan 30, 2026

hebasto force-pushed the 260129-iwyu-util branch from deca8eb to 695facc Compare January 30, 2026 21:25

hebasto force-pushed the 260129-iwyu-util branch from 695facc to 6d800fe Compare January 31, 2026 09:13

hebasto force-pushed the 260129-iwyu-util branch from d743c29 to 639cdba Compare February 10, 2026 15:55

DrahtBot removed the Needs rebase label Feb 10, 2026

hebasto mentioned this pull request Feb 10, 2026

ci: Extend diff context for clang-format #34551

Merged

fanquake reviewed Feb 10, 2026

View reviewed changes

src/util/time.cpp Show resolved Hide resolved

fanquake reviewed Feb 10, 2026

View reviewed changes

src/util/threadinterrupt.h Show resolved Hide resolved

hebasto force-pushed the 260129-iwyu-util branch from 639cdba to edbd916 Compare February 10, 2026 19:20

DrahtBot mentioned this pull request Feb 12, 2026

refactor: use _MiB/_GiB consistently for byte conversions #34435

Open

hebasto force-pushed the 260129-iwyu-util branch from edbd916 to 8cb1a63 Compare February 13, 2026 14:11

DrahtBot added the CI failed label Feb 13, 2026

hebasto force-pushed the 260129-iwyu-util branch from 8cb1a63 to 76762de Compare February 13, 2026 15:25

DrahtBot removed the CI failed label Feb 13, 2026

DrahtBot mentioned this pull request Feb 16, 2026

bench: use deterministic HexStr payload #34598

Open

fanquake reviewed Feb 18, 2026

View reviewed changes

src/util/fs.h Show resolved Hide resolved

hebasto added 3 commits February 18, 2026 16:29

iwyu: Drop IWYU pragma: export from src/sync.h

3469233

ci, iwyu: Fix warnings in src/util and treat them as errors

adcaac7

hebasto force-pushed the 260129-iwyu-util branch from 76762de to adcaac7 Compare February 18, 2026 18:43

fanquake reviewed Feb 18, 2026

View reviewed changes

hebasto mentioned this pull request Feb 20, 2026

iwyu: Document or remove some pragma: export and other improvements #34639

Open

hebasto marked this pull request as draft February 20, 2026 17:54

DrahtBot mentioned this pull request Feb 26, 2026

node: scale default -dbcache with system RAM #34641

Open

DrahtBot mentioned this pull request Mar 11, 2026

threading: never require logging from sync.h #34793

Closed

		@@ -8,6 +8,7 @@

		#include <crypto/siphash.h>
		#include <logging/categories.h> // IWYU pragma: export

Conversation

hebasto commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

fanquake commented Jan 29, 2026

Uh oh!

DrahtBot commented Jan 29, 2026

Uh oh!

fanquake commented Jan 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hebasto commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Jan 31, 2026

Uh oh!

hebasto commented Feb 10, 2026

Uh oh!

hebasto commented Feb 10, 2026

Uh oh!

hebasto commented Feb 10, 2026

Uh oh!

hebasto commented Feb 10, 2026

Uh oh!

Uh oh!

Uh oh!

hebasto commented Feb 13, 2026

Uh oh!

DrahtBot commented Feb 13, 2026

Uh oh!

Uh oh!

fanquake Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

hebasto Feb 18, 2026

Choose a reason for hiding this comment

Uh oh!

fanquake Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Feb 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hebasto commented Jan 29, 2026 •

edited

Loading

DrahtBot commented Jan 29, 2026 •

edited

Loading

hebasto commented Jan 30, 2026 •

edited

Loading

fanquake Feb 19, 2026 •

edited

Loading