Skip to content

[Serialization] Load non-public dependencies of modules when testing is enabled #64522

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 2 commits into from
Mar 22, 2023

Conversation

xymus
Copy link
Contributor

@xymus xymus commented Mar 21, 2023

Non-public dependencies may be referenced from internal decls in a module with testing enabled. If that module is imported as testable by a client, the client can access those internal decls and references for which it needs to load the dependencies.

Ensure that we load transitive non-public dependencies from modules that enabled testing. To do so, we differentiate internal and fileprivate imports from implementation-only imports at the module-wide level to offer a different module loading strategy.

rdar://106514965


I'm suggesting something similar for implementation-only imports in #64509, where the dependency can only be optional as adding them as required could break projects.

xymus added 2 commits March 21, 2023 16:45
…ader

Let's centralize the logic deciding if we load a transitive dependency
on the client side and have the producer write the truth in the
swiftmodule.
…imports

Differentiate `internal` and `fileprivate` imports from
implementation-only imports at the module-wide level to offer a
different module loading strategy. The main difference is for non-public
imports from a module with testing enabled to be loaded by transitive
clients.

Ideally, we would only load transitive non-public dependencies on
testable imports of the middle module. The current module loading logic
doesn't allow for this behavior easily as a module may be first loaded
for a normal import and extra dependencies would have to be loaded on
later imports. We may want to refactor the module loading logic to allow
this if needed.

rdar://106514965
@xymus xymus requested review from bnbarham and artemcm March 21, 2023 23:53
@xymus
Copy link
Contributor Author

xymus commented Mar 21, 2023

@swift-ci Please smoke test

Copy link
Contributor

@artemcm artemcm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@xymus
Copy link
Contributor Author

xymus commented Mar 22, 2023

@swift-ci Please smoke test

@xymus xymus merged commit cc43326 into swiftlang:main Mar 22, 2023
@xymus xymus deleted the access-level-import-testable branch March 22, 2023 19:54
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