[5.9][Serialization] Load non-public transitive dependencies on @testable imports #64794
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
…teWithFileContext
The new diagnoseSerializedASTLoadFailureTransitive diagnose problems for transitive dependencies only: missing dependency, missing underlying module, or circular dependency.
…imports A @testable import allows a client to call internal decls which may refer to non-public dependencies. To support such a use case, load non-public transitive dependencies of a module when it's imported @testable from the main module. This replaces the previous behavior where we loaded those dependencies for any modules built for testing. This was risky as we would load more module for any debug build, opening the door to a different behavior between debug and release builds. In contrast, applying this logic to @testable clients will only change the behavior of test targets. rdar://107329303
…dependencies When using the -testable-import-module argument to insert a testable import, there's no ImportDecl on which to show the diagnostics when loading transitive dependencies. Clean up the logic to still load dependencies in such a case.
…ependencies * A multi-file scenario where one file has a normal import and the other a testable import. In this case we still load the extra dependencies and show diagnostics only of the testable import. * A scenario where module A @testable imports module B, which @_exported imports module C. In this case, non-public imports of B are loaded, but not those of C. This is a limitation of the current implementation and could be improved upon in the future.
|
@swift-ci Please test |
nkcsgexi
approved these changes
Mar 30, 2023
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.
A
@testableimport allows a client to call internal decls which may refer to non-public dependencies. To support such a use case, load non-public transitive dependencies of a module when it's imported@testablefrom the main module.We apply this behavior only to
@testableimports from the main module being built, not transitively trough@testableimports of a library. It should support the vast majority of@testableuses and provide a reasonable behavior without surprises.This replaces the previous behavior where we loaded those dependencies for any modules built for testing. This would load more modules for any debug build, opening the door to a different behavior between debug and release builds. In contrast, applying this logic to
@testableclients will only change the behavior of test targets.rdar://107329303
Cherry-pick of the feature from #64693, plus as support: #64516 and #64522.
This affects only the new kind of imports using access level, as it's still behind a flag it should be low risk and not affect existing projects.