ci, iwyu: Fix warnings in `src/script` and treat them as errors #35011

pull BrandonOdiwuor wants to merge 2 commits into bitcoin:master from BrandonOdiwuor:iwyu-script changing 26 files +120 −39

BrandonOdiwuor commented at 8:14 AM on April 6, 2026: contributor

This PR continues the ongoing effort to enforce IWYU warnings.

See Developer Notes.
DrahtBot commented at 8:15 AM on April 6, 2026: contributor


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



Reviews

See the guideline for information on the review process.

Type Reviewers

Concept ACK hebasto, kevkevinpal

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #34806 (refactor: logging: Various API improvements by ajtowns)
- #34520 (refactor: Add [[nodiscard]] to functions returning bool+mutable ref by maflcko)
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.
hebasto commented at 8:48 AM on April 6, 2026: member
Concept ACK.

d41095a70d87e84b8c5cdfcefe1c53174fbabac6

Per our policy, "a comment explaining the rationale is required for every use of IWYU pragma: export":
```
$ git grep -l "IWYU pragma: export" -- src/script
src/script/interpreter.h
src/script/script.h
```
kevkevinpal commented at 4:35 PM on April 6, 2026: contributor

Concept ACK

Looks like common/ compat/ support/ are now left from that list after this PR is merged
BrandonOdiwuor commented at 12:58 PM on April 9, 2026: contributor

@kevkevinpal from the list I only think only compat/ and support/ are left there is already a PR: 34995 for common/ -> #34995. I don't know if this will be done for QT #33725 (comment)
BrandonOdiwuor renamed this:
~~ci, iwyu: Fix warnings in src/script and treat them as errors~~
ci, iwyu: Fix warnings in 'src/script' and treat them as errors
on Apr 9, 2026
BrandonOdiwuor renamed this:
~~ci, iwyu: Fix warnings in 'src/script' and treat them as errors~~
ci, iwyu: Fix warnings in `src/script` and treat them as errors
on Apr 9, 2026
hebasto commented at 1:16 PM on April 9, 2026: member

I don't know if this will be done for QT #33725 (comment)

It will be, though it requires some preliminary work.
ci, iwyu: Fix warnings in src/scripts and treat them as error a69d0a1568
iwyu: remove #include <coins.h> from src/script/sign.h as recommended by iwyu 737a165d9a
BrandonOdiwuor force-pushed on Apr 10, 2026
BrandonOdiwuor commented at 8:25 AM on April 10, 2026: contributor

Per our policy, "a comment explaining the rationale is required for every use of IWYU pragma: export":

updated the PR to include comments for the left IWYU pragma: export on scr/script

Type	Reviewers
Concept ACK	hebasto, kevkevinpal

in ci/test/03_test_script.sh:212 in 737a165d9a

 208 | @@ -209,7 +209,7 @@ fi
 209 |  
 210 |  if [[ "${RUN_IWYU}" == true ]]; then
 211 |    # TODO: Consider enforcing IWYU across the entire codebase.
 212 | -  FILES_WITH_ENFORCED_IWYU="/src/(((crypto|index|kernel|primitives|univalue/(lib|test)|util|zmq)/.*|common/license_info|node/blockstorage|node/utxo_snapshot|clientversion|core_io|signet)\\.cpp)"
 213 | +  FILES_WITH_ENFORCED_IWYU="/src/(((crypto|index|kernel|primitives|univalue/(lib|test)|util|zmq|script)/.*|common/license_info|node/blockstorage|node/utxo_snapshot|clientversion|core_io|signet)\\.cpp)"

hebasto commented at 11:42 AM on April 13, 2026:

nit: Could the subdirectories be lexicographically ordered?

in src/script/descriptor.cpp:28 in 737a165d9a

  23 |  #include <script/signingprovider.h>
  24 |  #include <script/solver.h>
  25 | +#include <serialize.h>
  26 | +#include <tinyformat.h>
  27 |  #include <uint256.h>
  28 |

hebasto commented at 11:43 AM on April 13, 2026:

nit: Drop empty line?

in src/script/interpreter.cpp:17 in 737a165d9a

   9 | @@ -10,8 +10,16 @@
  10 |  #include <crypto/sha256.h>
  11 |  #include <pubkey.h>
  12 |  #include <script/script.h>
  13 | +#include <serialize.h>
  14 | +#include <span.h>
  15 |  #include <tinyformat.h>
  16 |  #include <uint256.h>
  17 | +#include <algorithm>

hebasto commented at 11:44 AM on April 13, 2026:

Separate header groups by an empty line?

in src/script/interpreter.h:15 in 737a165d9a

   8 | @@ -9,21 +9,26 @@
   9 |  #include <consensus/amount.h>
  10 |  #include <hash.h>
  11 |  #include <primitives/transaction.h>
  12 | +#include <script/script.h>
  13 | +// script_error.h is exported because there should rarely be a case where it is
  14 | +// included without interpreter.h
  15 |  #include <script/script_error.h> // IWYU pragma: export

hebasto commented at 11:57 AM on April 13, 2026:

I see this explanation stems from faf1fb207fb6e9a12c864074f8c40d5922d93ff4, but saving a line feels like a weak rationale here. Let's stick to the ones that are documented in the Developer Notes or update them if needed.

cc @maflcko

in src/script/interpreter.h:18 in 737a165d9a

  13 | +// script_error.h is exported because there should rarely be a case where it is
  14 | +// included without interpreter.h
  15 |  #include <script/script_error.h> // IWYU pragma: export
  16 | +// verify_flags.h is exported because it defines the common SCRIPT_VERIFY_*
  17 | +// type alias used by the interpreter interface
  18 |  #include <script/verify_flags.h> // IWYU pragma: export

hebasto commented at 11:58 AM on April 13, 2026:

cc @ajtowns

in src/script/miniscript.cpp:6 in 737a165d9a

   1 | @@ -2,17 +2,16 @@
   2 |  // Distributed under the MIT software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | -#include <limits>
   6 | -#include <vector>
   7 | -
   8 | -#include <primitives/transaction.h>
   9 |  #include <script/miniscript.h>
  10 | +#include <primitives/transaction.h>

hebasto commented at 12:00 PM on April 13, 2026:

We usually separate the associated header by an empty line:

#include <script/miniscript.h>

#include <primitives/transaction.h>

in src/script/script.h:12 in 737a165d9a

   7 | @@ -8,14 +8,16 @@
   8 |  
   9 |  #include <attributes.h>
  10 |  #include <crypto/common.h>
  11 | +// prevector.h is exported because it exists primarily to represent scripts
  12 |  #include <prevector.h> // IWYU pragma: export

hebasto commented at 12:02 PM on April 13, 2026:

This comment stems from fa5ccc4137fdd14a75a6fc860b8ff6fc455cb55d. I have the same concerns as above.

cc @maflcko

in src/script/sign.cpp:26 in 737a165d9a

  22 | +#include <serialize.h>
  23 |  #include <uint256.h>
  24 | +#include <util/check.h>
  25 |  #include <util/translation.h>
  26 |  #include <util/vector.h>
  27 | +#include <algorithm>

hebasto commented at 12:03 PM on April 13, 2026:

nit: Separate groups by an empty line?

hebasto commented at 12:05 PM on April 13, 2026: member

Left some comments.

Contributors

Linked (view graph)

#33725 ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`#35103 ci, iwyu: Fix warnings in `src/compat/` & `src/support/` and treat them as error

ci, iwyu: Fix warnings in `src/script` and treat them as errors #35011

Reviews

Conflicts