PCH: Fix logic error that incorrectly clears sources during VS generation
Since !3762 (merged), GetPchFileObject handles the case that
it is called first for another target's REUSE_FROM by calling
AddSource to make sure GetObjectName can produce the correct object
name. However, AddSource causes ClearSourcesCache to be called,
which since !4751 (merged) now correctly erases the
AllConfigSources structure. This is okay during AddPchDependencies,
but there is another code path in which it is problematic.
When the Visual Studio generator's WriteAllSources method is looping
over the sources, the cmake_pch.cxx source is encountered first. This
causes OutputSourceSpecificFlags to call GetPchCreateCompileOptions,
which calls GetPchFile, which under MSVC with CMAKE_LINK_PCH calls
GetPchFileObject. That leads to ClearSourcesCache erasing the
structure over which WriteAllSources is iterating!
This bug is caught by our RunCMake.PrecompileHeaders test when run
with the VS generator as of the commit that exposed it by fixing
ClearSourcesCache. However, that change was backported to the CMake
3.16 series after testing only with later versions versions that contain
!4201 (merged). By adding proper multi-config support
for PCH, that merge request taught cmLocalGenerator::AddPchDependencies to
call GetPchFile with the real set of configurations instead of just
the empty string. This allows the GetPchFile cache of PCH sources to
be populated up front so that the later calls to it in the
WriteAllSources loop as described above do not actually call
GetPchFileObject or ClearSourcesCache. That hid the problem.
Fix this by re-ordering calls to AddPchDependencies to handle
REUSE_FROM targets only after the targets whose PCH they re-use.
Remove the now-unnecessary call to AddSource from GetPchFileObject
so that ClearSourcesCache is never called during WriteAllSources.
Update the PchReuseFrom test case to cover an ordering of targets that
causes generators to encounter a REUSE_FROM target before the target
whose PCH it re-uses.
Fixes: #20770 (closed)
Backport: release