-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Don't print extensions to conform to protocols that aren't printed #24788
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
This lets us make some more assumptions in the next commit, but I think it's also just a nice cleanup to /not/ allow random predicates here. There were three callers of this API: - PrintAST, which was using PrintOptions::shouldPrint but /also/ incorrectly notifying listeners that a declaration would be skipped. - (IDE) Interface generation, which uses PrintOptions::shouldPrint to count how many "inherits" there will be. - SwiftDocSupport's reportRelated, which does no filtering at all. Creating a PrintOptions here is a little more expensive, but still. No intended functionality change.
Try a little harder to avoid printing empty extensions by seeing if any of the inherited protocols are actually going to be printed. Previously this just made things a little prettier, but with implementation-only imports it's a correctness issue, since there may be extensions of implementation-only types that do in fact conform to non-public protocols. rdar://problem/50748072
@swift-ci Please test |
Ugh, didn't mean to put this in the main repo either, but it's already running tests… |
Build failed |
iOS sim failure looks unrelated, but I'll wait to see if more bots hit it before restarting. |
@swift-ci Please test macOS |
Build failed |
This time an unrelated failure in LLDB. @swift-ci Please test macOS |
rintaro
approved these changes
May 15, 2019
beccadax
approved these changes
May 15, 2019
jrose-apple
added a commit
to jrose-apple/swift
that referenced
this pull request
May 16, 2019
Don't print extensions to conform to protocols that aren't printed rdar://problem/50748072 (cherry picked from commit 6caedcd)
jrose-apple
added a commit
that referenced
this pull request
May 16, 2019
Don't print extensions to conform to protocols that aren't printed rdar://problem/50748072 (cherry picked from commit 6caedcd)
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.
Try a little harder to avoid printing empty extensions by seeing if any of the inherited protocols are actually going to be printed. Previously this just made things a little prettier, but with implementation-only imports it's a correctness issue, since there may be extensions of implementation-only types that do in fact conform to non-public protocols.
rdar://problem/50748072