-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[SE-0491] Implement lookup and diagnostics for module selectors (MyMod::someName) #34556
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
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
eb8ecdf to
b6d41f6
Compare
beccadax
commented
Jul 21, 2021
9c9e8e9 to
6edf769
Compare
Contributor
|
Woohoo! Thank you for reviving this, Becca! |
Contributor
Author
|
@swift-ci test |
Contributor
Author
|
@swift-ci test |
beccadax
commented
Oct 23, 2025
Comment on lines
+300
to
+303
| // Disable module selector filtering--UnqualifiedLookupRequest should have | ||
| // done it. | ||
| LookupResultBuilder builder(result, dc, /*moduleSelector=*/Identifier(), | ||
| options); |
Contributor
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slavapestov This is the result of that review comment you made on the other PR.
tshortli
approved these changes
Oct 23, 2025
Contributor
Author
|
@swift-ci please test |
In this commit, this change affects certain diagnostics but doesn’t actually alter name lookup behavior yet.
Adds comments explaining why these conversions aren’t lossy.
Lookups like Builtin::Int64 were failing because BuiltinUnit rejected all unqualified lookups. Make it allow unqualified lookups with a module selector.
In code like the following:
```
protocol P { associatedtype A: Hashable }
protocol Q { associatedtype A: Comparable }
func fn<T: P & Q>(_: T) where T.A == Int { … }
```
`T.A` is actually the union of `P.A` and `Q.A`—it satisfies both associated types and has both of their constraints. This means it doesn’t actually make sense to apply a module selector to `A`—even if `P` and `Q` are in different modules, `T.A` always represents both of the declarations, not one or the other. We therefore now ban module selectors in this position, since they don’t actually jibe with the nature of a generic signature.
This justification technically doesn’t hold for *every* member type of a generic parameter—a member type can refer to a concrete typealias in a protocol extension, for instance—but in those situations, you can disambiguate (and add module selectors) by writing `P.A` or `Q.A` instead of `T.A`, so we’re not really worried about this limitation.
Approved by SE-0491.
Contributor
Author
|
@swift-ci please smoke test |
Contributor
|
🎉 |
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 pull request implements the lookup behavior, including tailored diagnostics, for "module selectors", which allow references to declarations to be unambiguously prefixed with a module they can be found through:
Since SE-0491 has been approved and this PR is sufficient to make them functional, it also changes them from an experimental feature to an ordinary on-by-default language feature.
Parsing for this feature has already landed in #84362. Conditional emission into module interface files will land in a future PR.
This work was previously in PR #28834, which was closed during the master-to-main transition.
Fixes rdar://problem/19481048.