Skip to content

[6.2 🍒][Dependency Scanning] Fix search path context hashing hole and avoid serializing unnecessary field #81255

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 5, 2025

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented May 2, 2025

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 auxiliaryFiles from 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 auxiliaryFiles is expected to be a no-op.
Testing: Added tests to test suite
Original PRs: #81244
Reviewers: @cachemeifyoucan

artemcm added 3 commits May 2, 2025 09:52
…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
@artemcm artemcm requested a review from nkcsgexi May 2, 2025 17:08
@artemcm artemcm requested a review from a team as a code owner May 2, 2025 17:08
@artemcm
Copy link
Contributor Author

artemcm commented May 2, 2025

@swift-ci test

@artemcm
Copy link
Contributor Author

artemcm commented May 2, 2025

@swift-ci test Windows platform

@artemcm artemcm merged commit bb581a8 into swiftlang:release/6.2 May 5, 2025
5 checks passed
@artemcm artemcm deleted the IncrementalScanFixes_62 branch May 5, 2025 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants