Skip to content

[Sema] Handle package exportability. #73161

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
May 3, 2024
Merged

[Sema] Handle package exportability. #73161

merged 2 commits into from
May 3, 2024

Conversation

elsh
Copy link
Contributor

@elsh elsh commented Apr 20, 2024

This PR treats package access level as exportable, preventing
internally imported types from accidentally being declared in
package decl signatures.

Added package-specific cases to ExportabilityReason and
DisallowedOriginKind to track the validity of imported types
at use sites with package access scope. Added tests to cover
variety of use cases.

Resolves rdar://117586046&125050064&124484388&124306642

@elsh elsh requested review from xymus, nkcsgexi and artemcm April 20, 2024 01:51
@elsh elsh changed the title Handle package exportability. [Sema] Handle package exportability. Apr 20, 2024
@elsh
Copy link
Contributor Author

elsh commented Apr 20, 2024

@swift-ci smoke test

@xymus
Copy link
Contributor

xymus commented Apr 23, 2024

There are a few failures in Sema/access-level-import-classic-exportability.swift that appear to be expected new diagnostics from exportability checking. It would be worthwhile to write a test that reproduces them with a simple package decl without the package import.

e.g.

.../Client.swift:66:76: error: unexpected error produced: cannot use struct 'PackageImportType' in an extension with conditional conformances; 'PackageLib' was not imported publicly
extension Array: PublicConstrainedExtensionProtoInPackage where Element == PackageImportType {}
                                                                           ^

This one may need an update to the error message:

.../Client.swift:157:11: error: unexpected error produced: cannot use struct 'FileprivateImportType' in an extension with public or '@usableFromInline' members; 'FileprivateLib' was not imported publicly
extension FileprivateImportType {
          ^

Can we also test that we're not allowing package decls in public inlinable code?

@elsh elsh force-pushed the elsh/pkg-export branch 2 times, most recently from ab17a0f to d9dec1f Compare May 2, 2024 10:45
@elsh
Copy link
Contributor Author

elsh commented May 2, 2024

@swift-ci smoke test

This PR treats package access level as exportable, preventing
internally imported types from accidentally being declared in
package decl signatures.

Added package-specific cases to ExportabilityReason and
DisallowedOriginKind to track the validity of imported types
at use sites with package access scope. Added tests to cover
variety of use cases.

Resolves rdar://117586046&125050064&124484388&124306642
@elsh elsh force-pushed the elsh/pkg-export branch from d9dec1f to d182d01 Compare May 2, 2024 12:17
@elsh
Copy link
Contributor Author

elsh commented May 2, 2024

@swift-ci smoke test

@elsh
Copy link
Contributor Author

elsh commented May 2, 2024

@swift-ci smoke test

@elsh
Copy link
Contributor Author

elsh commented May 3, 2024

This now affects projects using package with the previous exportability rules; CI pending updates in swiftpm swiftlang/swift-package-manager#7525

MaxDesiatov pushed a commit to swiftlang/swift-package-manager that referenced this pull request May 3, 2024
)

The exportability rules around `package` access level are updated in a
way that an `@_spi public` can't be bound to a `package` var, similar to
how `@_spi public` can't be bound to a `public` var. For reference, see
swiftlang/swift#73161.
This PR corrects such use case.
@elsh
Copy link
Contributor Author

elsh commented May 3, 2024

@swift-ci smoke test

@@ -2324,7 +2324,7 @@ bool VarDecl::isLayoutExposedToClients() const {
auto nominalAccess =
parent->getFormalAccessScope(/*useDC=*/nullptr,
/*treatUsableFromInlineAsPublic=*/true);
if (!nominalAccess.isPublic()) return false;
if (!nominalAccess.isPublicOrPackage()) return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: move return false to a new line for better debuggability.

@elsh elsh merged commit 1c012c8 into main May 3, 2024
@elsh elsh deleted the elsh/pkg-export branch May 3, 2024 21:48
ahoppen added a commit to swiftlang/swift-package-manager that referenced this pull request May 15, 2024
)

- **Explanation**: Cherry-pick swiftlang/swift#73161
to `releases/6.0` so that SwiftPM 6.0 can be built with a `main` Swift
development snapshot
- **Scope**: Changes access level of a member from `package` to SPI
- **Risk**: Very low, just changes the access lavel 
- **Testing**: n/a
- **Issue**: n/a
- **Reviewer**: @MaxDesiatov and @bnbarham on
#7525
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
…iftlang#7525)

The exportability rules around `package` access level are updated in a
way that an `@_spi public` can't be bound to a `package` var, similar to
how `@_spi public` can't be bound to a `public` var. For reference, see
swiftlang/swift#73161.
This PR corrects such use case.
furby-tm pushed a commit to wabiverse/swift-package-manager that referenced this pull request May 15, 2024
…iftlang#7525)

The exportability rules around `package` access level are updated in a
way that an `@_spi public` can't be bound to a `package` var, similar to
how `@_spi public` can't be bound to a `public` var. For reference, see
swiftlang/swift#73161.
This PR corrects such use case.
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.

3 participants