Drop some IWYU pragma: export and document IWYU usage
#34319
Conversation
`IWYU pragma: export` enforces the transitive inclusion of the headers, which undermines the purpose of IWYU. The remained cases seem useful and could be considered separately: - `<cassert>` in `util/check.h` - `<filesystem>` in `util/fs.h` - `<chrono>` in `util/time.h`
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34319. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please copy-paste |
|
review ACK 923d9ec 🗝 Show signatureSignature: |
doc/developer-notes.md
Outdated
| The [`include-what-you-use`](https://github.com/include-what-you-use/include-what-you-use) tool (IWYU) | ||
| helps to enforce the source code organization [policy](#source-code-organization) in this repository. | ||
|
|
||
| When running IWYU locally, it is recommended to use the same version as the [CI](/ci/test/00_setup_env_native_iwyu.sh) |
There was a problem hiding this comment.
Does this actually work? Doesn't the standard library/compiler version also need to match? I'm guessing it won't work for anyone on macOS?
I would think it'd be better to only recommended using the CI job (locally), so it's guaranteed to produce the same results.
doc/developer-notes.md
Outdated
| to a function that accepts a `std::string` parameter: an implicit conversion occurs at the callsite using the | ||
| `std::string` constructor, which makes the corresponding header required. | ||
|
|
||
| We accept these suggestions, and no further action is required to silence them. |
There was a problem hiding this comment.
If the suggestion has been applied, there shouldn't be anything left to silence?
923d9ec to
d9c244d
Compare
|
The recent feedback from @fanquake has been addressed. |
First two commits address comments from discussion in #33779:
src/kerneland treat them as errors #33779 (comment)src/kerneland treat them as errors #33779 (comment)The last commit adds a new section to the Developer Notes to document IWYU usage.