Skip to content

Use access levels on imports when compiler(>=6) #2917

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 1 commit into from
Dec 14, 2024

Conversation

tshortli
Copy link
Contributor

@tshortli tshortli commented Dec 13, 2024

Access levels on import declarations are a language feature that was introduced with the Swift 6 compiler but they don't depend on the Swift 6 language mode. @_implementationOnly import is now deprecated and internal import should be used instead, so SwiftSyntax's use of access levels on imports should depend on the compiler version, not the language version, to ensure it no longer uses @_implementationOnly unnecessarily.

As a notable exception, _SwiftSyntaxCShims must continue to be imported @_implementationOnly since it's a local module that cannot be loaded by clients, and internal import does not hide modules from dependents when SwiftSyntax is built without library evolution.

@tshortli
Copy link
Contributor Author

We should also consider enabling the upcoming InternalImportsByDefault feature regardless of language mode, but I think that should be done in a separate PR.

@tshortli tshortli force-pushed the access-level-on-imports branch 2 times, most recently from 681cdcc to 1464815 Compare December 13, 2024 18:58
@tshortli
Copy link
Contributor Author

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks, Allan.

@ahoppen
Copy link
Member

ahoppen commented Dec 13, 2024

@swift-ci Please test Windows

@tshortli tshortli force-pushed the access-level-on-imports branch from 1464815 to 5d70fe6 Compare December 13, 2024 21:30
@tshortli
Copy link
Contributor Author

I forgot to update code generation - fixed.

@tshortli
Copy link
Contributor Author

@swift-ci please test

@tshortli
Copy link
Contributor Author

Alright, as it turns out access level on import still requires a feature flag on 5.10. So I will update the #if compiler checks to check for the Swift 6 compiler.

Access levels on `import` declarations are a language feature that was
introduced with the Swift 6 compiler but they don't depend on the Swift 6
language mode. `@_implementationOnly import` is now deprecated and `internal
import` should be used instead, so SwiftSyntax's use of access levels on
imports should depend on the compiler version, not the language version to
ensure it no longer uses the `@_implementationOnly` unnecessarily.

As a notable exception, `_SwiftSyntaxCShims` must continue to be imported
`@_implementationOnly` since it's a local module that cannot be loaded by
clients, and `internal import` does not hide modules from dependents when
SwiftSyntax is built without library evolution.
@tshortli tshortli force-pushed the access-level-on-imports branch from 5d70fe6 to 1d1aded Compare December 13, 2024 23:16
@tshortli tshortli changed the title Use access levels on imports when compiler(>=5.10) Use access levels on imports when compiler(>=6) Dec 13, 2024
@tshortli
Copy link
Contributor Author

@swift-ci please test

@tshortli tshortli merged commit 8c832fe into swiftlang:main Dec 14, 2024
3 checks passed
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