Fix non-public imported declaration diagnostic bug + Favor private imports during name lookup #33864
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
|
(#33716 will conflict with this as soon as it lands, but the changes to resolve that are trivial.) |
beccadax
force-pushed
the
imported-by-the-east-aldenard-trading-company
branch
from
9c2a26c to
437e487
Compare
|
Build failed |
|
Build failed |
|
@swift-ci please test |
|
Build failed |
beccadax
force-pushed
the
imported-by-the-east-aldenard-trading-company
branch
from
437e487 to
2950f2d
Compare
|
Force-push to fix some case-sensitivity and path-separator issues in one of the tests. |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
slavapestov
approved these changes
Sep 23, 2020
When DiagnosticEngine needs to diagnose something about an imported declaration, it uses ASTPrinter to print the declaration’s interface into a source buffer and then diagnoses it there. However, this code only printed public declarations, so it failed to account for features like `@testable import` which allow less-than-public declarations to be imported. Errors involving these declarations would therefore be diagnosed at <unknown>:0. This commit changes DiagnosticEngine to determine the access level of the declaration it needs to print and, if it is below `Public`, instead prints a separate interface whose minimum access level is low enough to include the desired declaration.
Private imports are intended to make the file performing the import more or less source-compatible with the file being imported from, so that code from the original file can be modified by relatively simple syntactic transformations. However, their name shadowing behavior was very different from the original file. In the original file, other declarations in the same module would have shadowed declarations imported from any other module; in a file using a @_private import, they would all be considered imports, and thus all would be preferred equally. This could cause ambiguity in a file using a @_private import that was not present in the original file. This commit changes that behavior by favoring @_private imports over other imports, so that if a name is visible through both a private and a non-private import, the one visible through the private import will shadow the other. This shadowing takes a higher priority than a scoped import, but a lower priority than the check for whether one of the modules is only visible through the other. Fixes rdar://68312053.
beccadax
force-pushed
the
imported-by-the-east-aldenard-trading-company
branch
from
2950f2d to
806125d
Compare
|
Rebased onto @swift-ci please smoke test |
|
@swift-ci please smoke test |
|
Seems to have missed Windows. Trying again: @swift-ci please smoke test |
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 contains two changes; the first fixes a bug discovered while writing the tests for the second.
@_private(sourceFile:) importshould, to the extent practicable, preserve source compatibility with the original file. However, name lookup ended up working very differently. In the original file, declarations in the same module would shadow declarations from any other module; in the file importing it, the declarations from the original file's module would be considered just as imported as any other module's, so there would be new name lookup ambiguities. To address this, we add a shadowing rule which favors modules with a private import in the DeclContext's file over other modules visible from that file.The first change is a pure bug fix. The second is a behavior change, but in a compiler-internal feature.