Add missing includes for non-unity / no-PCH builds - #136
Open
itismyfield wants to merge 1 commit into
Open
Conversation
Unity builds and the shared PCH hide these dependencies: each translation unit picks the declarations up from a neighbour in the same unity blob, or from the PCH, so nothing complains. Building with unity disabled and PCHs off fails in 106 files. Includes only. No deletions, no behavioural change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
itismyfield
force-pushed
the
iwyu-nonunity
branch
from
August 11, 2026 06:47
c0403c7 to
8d420e3
Compare
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
153
#includelines across 106 files. No deletions, no code changes, no behavioural change.Why
Unity builds hide missing includes: several
.cppfiles are concatenated into one translation unit, so a file silently gets the declarations it needs from whichever neighbour happened to include them. A shared PCH covers most of the rest.Building with unity disabled and PCHs off surfaces them — 106 files fail to compile because they name types they never include.
-DisableUnityon its own does not catch these, since the PCH still supplies the headers; that is how a unity-off build gate can stay green while the files are still not self-contained.This is the include-what-you-use convention the engine's own modules follow, so each fix is mechanical: add the header that declares a type the file already uses.
Reproducing
In a module's
Build.cs:Most frequent additions:
AssetRegistry/AssetData.hPolicies/CondensedJsonPrintPolicy.hDom/JsonObject.hModules/ModuleManager.hUObject/TextProperty.h,UObject/Package.h,Serialization/JsonWriter.h,Serialization/JsonSerializer.h,Math/RandomStream.hBy module: MonolithMesh 24, MonolithIndex 21, MonolithCore 12, MonolithAI 9, MonolithLogicDriver 7, MonolithUI 6, MonolithGAS 5, MonolithAudio 5, remainder 1–3 each.
Two placements worth a look
Both files already had the header, but in a preprocessor branch that does not cover the use site — so these went inside a branch rather than at the top of the file:
MonolithMeshHandlePool.cpp—ListHandles()in the!WITH_GEOMETRYSCRIPTstub branch returnsMakeShared<FJsonObject>(), while the file's onlyDom/JsonObject.hsits in the#else. Added inside the stub branch, to avoid a redundant include on the GeometryScript path.MonolithLogicDriverInternal.cpp—AssetRegistry/AssetData.hadded inside#if WITH_LOGICDRIVER, alongside the existing include group, since every use site is in that block.Verification
The bulk of the set was established against v0.21.2 by compiling each module with unity off and PCHs off on UE 5.7, iterating until clean, then rebased onto
master. The rebase touched one file —MonolithBlueprintActions.cpp, where a later commit added an include at the same position.Two of the 106 are new files on
masterrather than carried forward, and they are direct evidence for the PCH point above:MonolithIndexRecoveryTests.cppandMonolithProjectSearchTests.cppboth callFPlatformFileManager::Get().GetPlatformFile().DeleteFile(...)whileHAL/PlatformFileManager.honly forward-declaresIPlatformFile, so a no-PCH build fails them withBoth now include
GenericPlatform/GenericPlatformFile.h, matching whatMonolithIndexDatabase.cppandMonolithSourceDatabase.cppneed for the same call. These two were found by compilingmaster's sources; the remaining 104 were not recompiled onmaster, so read those as "verified on 0.21.2 / UE 5.7, carried forward mechanically". No 5.8 build was run.If you would rather have a full no-PCH pass on
masterbefore merging, say so and I will redo it.