Run codebase through IWYU and add a CI check. ~7-10% improvements to build times.

[moxian]

Summary

None

Purpose of change

(This is a revival/continuation of #79239 following the advice from the comments therein.)
IWYU is a tool that checks that all the symbols used in C++ code are also actively #included. This is considered to be a good thing, some specific benefits being listed in their FAQ
This PR fixes [a decent chunk of the code] to include what it uses.
It also adds a CI job aimed to maintain the correct includes going forward, so as to prevent backsliding and the need for these massive sweeping PRs.

Describe the solution

• Add the CI job
• Add a mechanism to opt out certain files from being checked by the CI
• Opt out literally everything initially.
• Run IWYU on the "easy" code. Make the changes to please the tool.
  "Easy" here meaning "everything under tests/ and those files under src/ that have no define shenanigans in them whatsoever"
• Keep automated commits separate from manual fixes, and keep track of the code size metrics througout the process (as requested in the previous CI)

Code size

step	includes src/	includes tests/	clang -E src/	clang -E test/
initial	288068	171408	68567896	40671408
IWYU tests/	288068	167044	68567896	39341957
fixes	289634	168196	69096536	40499273
IWYU src/ --safe_headers	290436	168196	69365093	40499273
fixes	290441	168196	69365837	40499273
IWYU src/ --nosafe_headers	276953	162948	65627805	38287614
fixes	278247	163745	65903054	38434408
absolute difference start to finish	-9821	-7663	-2664842	-2237000
relative difference	-3.4 %	-4.5%	-3.9%	-5.5%

Here:

• the "includes" column means, roughly make includes && cat obj/*.inc | wc -l - i.e. the total size of recorded include.. paths? relations? dependencies? idk how to word this properly.
• the clang -E column representes, rougjly clang -E src/*.cpp | wc -l, i.e. the total size of the codebase after the preprocessor has run and #includes were, well, included.

The make includes was run wtih TILES=1 SOUND=1 RELEASE=1 LOCALIZE=1 BACKTRACE=1, the clang -E was run with flags from the equivalent cmake build. On WSL Ubuntu 24.04

Build times

I have measured build times without the change (matrix win) and with it (matrix win), basing on 9492421 with ccache disabled. Results:

	without IWYU	with IWYU	diff	delta
windows	43m 3s	39m 53s	-3m 10s	-7.4%
basic build	35m 9s	32m 6s	-3m 3s	-8.7%
gcc 9 lto	39m 15s	32m 23s	-6m 52s	-17.5%
clang 18 asan	47m 0s	42m 22s	-4m 38s	-9.9%
gcc 12 asan	60m 52s	55m 20s	-5m 32s	-9.1%
gcc 9 cmake	29m 17s	26m 30s	-2m 47s	-9.5%

Looks like 7-10% build time improvement (although that's perhaps not too impressive when it translates to only a couple of minutes absolute difference).

Future work

There is ample opportunity to improve:

• Roughly 30% of the cpp files under src/ are still opted out. Some of those are really low hanging fruit and would be straightforward to fix.
• The IWYU CI job currently runs on every PR, even json-only, but that would be easy to fix. Edit: should be fixed now
• The CI job currently runs on every cpp file, not only those affected by the change, and that would be slightly more tricky to fix, but still not too bad. The CI runtime is ~30minutes right now, I think this is acceptable in the short term.
• The documentation lacking.
• Only the barebones (TILES=0 SOUND=0 etc) configuration is tested for IWYU'ness.
• There's a minor issue with IWYU in that it recommends #include <stddef.h> for size_t which immediately triggers clang-tidy complaint to modernize stddef.h to cstddef. This is not fixable with IWYU config, but perhaps we can post-process its output and tweak the error message to the correct one.

Testing

CI

Additional context

Edit: Resolved, thanks Kevin!
There are pre-existing merge conflicts. I don't particularly want to waste more time on this if this PR is not something that's desired (because based on the previous interaction, I'm still not sure whether it is), so I won't be fixing those until there is some form of approval from whoever is willing to actually merge it.

(and also if there are requests to change the CI, it'd be easier to work on that first, and only then, after said requests are satisfied, fix the conflicts)

I don't have a PR with a failing IWYU CI handy anymore, but this is what the logs look like when it does make suggestions: https://github.com/moxian/Cataclysm-DDA/actions/runs/13210360655

For reviewers: This is perhaps more feasible to review commit-by-commit. I kept the mechanical changes separate from the manual ones, although I'm still not sure if that helps with the review.

CI status in my fork until (until CI here catches up) - moxian#22 ; https://github.com/moxian/Cataclysm-DDA/pull/22/checks?check_run_id=36989468855

If this PR is merged, then other PRs would likely get merge conflicted. I would suggest those accept all the headers, and ignore the IWYU suggestions/breakage to get unblocked, since it can get confusing. It can be fixed in a separate PR later.

[github-actions]

Auto-requesting reviews from non-collaborators: @jbytheway @wapcaplet @andrei8l

[kevingranade]

I can say up front that based on the magnitude of improvements in preprocessed files and build times, and the CI and opt-in workflows being included, I AM interested in getting this merged.

[akrieger]

The github web UI just yeets itself off a cliff on this PR. Can we PR some of the noncontroversial changes separately to help ease this in (eg. the c header modernization stuff can be trivally merged by itself).

[moxian]

Reviewing this commit-by-commit should help.
The modernize-headers is a response to the changes in previous commit (specifically "Mechanically IWYU the tests" one) since i didn't catch those in time. It is uselesss on its own (and unmergeable really)

There only three kinds of changes here:

• add the CI (first commit, reviewable in github web)
• two huge mechanical passes (unreviewable)
• manual fixups (reviewable again)

I'm not sure submitting any of those separately would help enough.

edit: well, i guess i can move out 28db853 and 0c8954a . But, again, i don't think this is quite enough to make things beter.

[akrieger]

Some quick review comments before you do too much more. I got down to around src/construction.cpp and find it unlikely I'll see something else generic to suggest.

Also maybe add to flexbuffer_json.h although I haven't seen it show up as an unnecessary addition much anywhere.

#include "json_error.h" // IWYU pragma: export

[moxian]

Minor issue i noticed: if there are no fatal parse errors, then IWYU exits with code 0 even if it made suggestions on how to adjust includes (see the current CI run: https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/13272432900/job/37054800929?pr=79631)

Easily fixable, but I shall declare this as a feature for now, for the transition period.

[akrieger]

Although I can't imagine how something might decide to barf at this point (if anything the Windows build would be most susceptible to header reordering), I'm sitting on the builds until they go green before merging.