[Serialization] Package imports #64047
Merged
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
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
elsh
reviewed
Mar 3, 2023
| Default = 1 << 1, | ||
| /// Include imports declared with `@_implementationOnly` or with an | ||
| /// access-level of `package` or less. | ||
| ImplementationOnly = 1 << 2, | ||
| /// Include imports of SPIs declared with `@_spi`. | ||
| SPIAccessControl = 1 << 3, |
There was a problem hiding this comment.
Shouldn't @_spi import still be allowed? Otherwise might be too source breaking no?
There was a problem hiding this comment.
We still support @_spi(X) import but we didn't really need that information in the filter. That filter is mostly used to call getImportedModules which allows mostly to get all public imports, all implementation-only imports, etc. In comparison, the logics specific to @_spi use some other filter and then checks for the spiGroups field in the returned AttributedImport.
That filter wasn't needed in practice, we can remove it.
This general test applies to all access-level. Let's precise that it tests the scenario where a client is outside of the package to preserve the current behavior. A new tests will check the behavior of a package dependency for clients in package.
…ackage When loading a swiftmodule A, read its package information to tell if the current client should load A's dependencies imports by a package import. Only clients belonging to the same package as A should load those dependencies, clients outside of the package likely don't have access to those dependencies. This is specific to swiftmodules as swiftinterfaces never display a package-only import. Clients are unaware of package dependencies when building against a swiftinterface. rdar://106164813
xymus
force-pushed
the
package-only-imports
branch
from
23fd0a7 to
4b14a8a
Compare
|
Adding the main module loading test that I forgot to commit before. @swift-ci Please smoke test |
elsh
approved these changes
Mar 3, 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.
This PR completes the support for package-only imports, aka package imports as it's spelled in source code with
package import SomePackageDependency. Package imports allow modules from a same package to have a shared dependency on a package-internal module, which enablespackagedecls to refer to types from that module.Previous PRs have added exportability checks for package imported decls (#64014 & #64033) and #63974 ensured that package imports are not printed in swiftinterfaces. This PR adds support for package imported dependencies in swiftmodule files.
The new behavior applies when loading a swiftmodule, let's say for module A, the compiler reads its package membership information to tell if the current client should load A's dependencies imported by a package import. Only clients belonging to the same package as A should load those dependencies, clients outside of the package likely don't have access to those dependencies. This applies only if A enabled library-evolution, without it all clients load the package dependencies.
See the following test for a typical use case of package import: test/Sema/package-only-references.swift
rdar://106164813