Manage the include hell

[patricklodder]

Follow-up from #3377

Problem statement

We often run into issues where we (or upstream) naively included or didn't include headers. This basically is hidden tech debt, and although over the past 3 years, we needed less than 5 fixes per year, these do block whomever is using newer compilers or dependencies than we release for, sometimes for prolonged time.

Potential solution

There's a tool called include-what-you-use (IWYU) that plugs into clang/llvm capabilities to detect inclusion issues. I've seen Bitcoin Core devs use this tool to fix up some of the hidden debt in the past too. Perhaps we could:

1. Create a contrib/devtools script that runs that tool over our codebase
2. Fix issues that it finds
3. If we manage to fix all historical issues, run that script in a CI lint job, so that we spot newly introduced issues with each PR

Scope

I'd suggest doing this first for 1.21, see how far we get with cleaning things up and get an idea about the cost of the operation, and then discuss and decide whether we want to do it for 1.14.

[patricklodder]

Example results when running iwyu with clang-14 against the files mentioned in #3377:

util/bip32.cpp

# python3 ../include-what-you-use/iwyu_tool.py src/util/bip32.cpp -p . -j6 -- -Xiwyu --cxx17ns

util/bip32.h should add these lines:
#include <stdint.h>      // for uint32_t

util/bip32.h should remove these lines:

The full include-list for util/bip32.h:
#include <attributes.h>  // for NODISCARD
#include <stdint.h>      // for uint32_t
#include <string>        // for string
#include <vector>        // for vector
---

util/bip32.cpp should add these lines:
#include <algorithm>            // for copy, max
#include <cstdint>              // for uint32_t

util/bip32.cpp should remove these lines:

The full include-list for util/bip32.cpp:
#include <util/bip32.h>
#include <stdio.h>              // for size_t
#include <tinyformat.h>         // for format, strprintf
#include <util/strencodings.h>  // for ParseUInt32
#include <algorithm>            // for copy, max
#include <cstdint>              // for uint32_t
#include <sstream>              // for basic_istream, stringstream
---

util/string.cpp

# python3 ../include-what-you-use/iwyu_tool.py src/util/string.cpp -p . -j6 -- -Xiwyu --cxx17ns

util/string.h should add these lines:
#include <stdint.h>      // for uint8_t

util/string.h should remove these lines:

The full include-list for util/string.h:
#include <attributes.h>  // for NODISCARD
#include <stdint.h>      // for uint8_t
#include <algorithm>     // for equal
#include <array>         // for array
#include <cstring>       // for strlen, size_t
#include <locale>        // for locale
#include <sstream>       // for ostringstream, basic_ios::imbue
#include <string>        // for string
#include <vector>        // for vector
---

(util/string.cpp has correct #includes/fwd-decls)

support/lockedpool.cpp

# python3 ../include-what-you-use/iwyu_tool.py src/support/lockedpool.cpp -p . -j6 -- -Xiwyu --cxx17ns

support/lockedpool.h should add these lines:
#include <stddef.h>       // for size_t

support/lockedpool.h should remove these lines:
- #include <stdint.h>  // lines 8-8

The full include-list for support/lockedpool.h:
#include <stddef.h>       // for size_t
#include <list>           // for list
#include <map>            // for multimap<>::const_iterator, multimap, multi...
#include <memory>         // for unique_ptr
#include <mutex>          // for call_once, mutex, once_flag
#include <unordered_map>  // for unordered_map
---

support/lockedpool.cpp should add these lines:
#include <ext/alloc_traits.h>  // for __alloc_traits<>::value_type
#include <limits>              // for numeric_limits
#include <stdexcept>           // for runtime_error
#include <utility>             // for pair, move

support/lockedpool.cpp should remove these lines:
- #include <config/bitcoin-config.h>  // lines 9-9
- #include <limits.h>  // lines 20-20

The full include-list for support/lockedpool.cpp:
#include <support/lockedpool.h>
#include <ext/alloc_traits.h>  // for __alloc_traits<>::value_type
#include <support/cleanse.h>   // for memory_cleanse
#include <sys/mman.h>          // for size_t, madvise, mlock, mmap, munlock
#include <sys/resource.h>      // for getrlimit, rlimit, RLIMIT_MEMLOCK, RLI...
#include <unistd.h>            // for sysconf, _SC_PAGESIZE
#include <algorithm>           // for min
#include <limits>              // for numeric_limits
#include <stdexcept>           // for runtime_error
#include <utility>             // for pair, move
---

[chromatic]

👍 from me. I tested the same tool a few weeks ago and it looked useful.

The first item in your list seems like a good first issue too, though it might need the disclaimer "(for someone familiar with compiling)".

[patricklodder]

#3454 is another example of this being useful - running iwyu over src/bench solved an issue that is (likely) caused by a boost 1.84 header no longer including <limits>