[6.2 🍒][Dependency Scanning] Fix search path context hashing hole and avoid serializing unnecessary field #81255
Conversation
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
…rialization - Deserialization of binary module dependencies was still relying on stale code (e.g. 'currentModuleImports', 'currentOptionalModuleImports') from serialized import strings, instead of the now in-use import infos. - Imports without a source location (e.g. implicit imports of stdlib) were not getting serialized at all - Optional import arrays were not being written out at all
The field is only used to store information to be used in finalize stage, in caching builds. When loading scan results from the cache, the entries are finalized already and have the file info encoded in CASIDs already. Resolves rdar://150307865
This hash is also used for the dependency scanning hash. In both cases, PCH contents may differ based on whether a certain module they depend on is found in a system or non-system search path. In dependency scanning, systemness should cause a full change of scanning context requiring a from-scratch scan. Resolves rdar://150334077
Contributor
Author
|
@swift-ci test |
nkcsgexi
approved these changes
May 2, 2025
Contributor
Author
|
@swift-ci test Windows platform |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Cherry-pick of #81244
Only the top two commits are new, the oldest commit in this PR is under standalone review in #81254
Explanation: This change addresses an issue in incremental dependency scanning where the scanner would incorrectly re-use prior scanning results if the prior scan was different from the current scan by having identical search paths specified as Non-System and the current scan has the same search paths specified as System, and vice versa. This is due to the PCH search path hash code, which is also used for the dependency scanning hash. In both cases, PCH contents may differ based on whether a certain module they depend on is found in a system or non-system search path. This change also removes a field
auxiliaryFilesfrom getting serialized. This field, in caching builds, is not required to be serialized.Problem: rdar://150307865 & rdar://150334077
Risk: Low, the change to the hashing function used in this PR is to make it more strict, in a way it should always have been, which should make it more sound. The change to remove serialization of
auxiliaryFilesis expected to be a no-op.Testing: Added tests to test suite
Original PRs: #81244
Reviewers: @cachemeifyoucan