Skip to content

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
merged 2 commits into from
May 16, 2019

Conversation

jrose-apple
Copy link
Contributor

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

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
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@jrose-apple
Copy link
Contributor Author

Ugh, didn't mean to put this in the main repo either, but it's already running tests…

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - def1cca

@jrose-apple
Copy link
Contributor Author

iOS sim failure looks unrelated, but I'll wait to see if more bots hit it before restarting.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please test macOS

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - def1cca

@jrose-apple
Copy link
Contributor Author

This time an unrelated failure in LLDB.

@swift-ci Please test macOS

@jrose-apple jrose-apple merged commit 6caedcd into master May 16, 2019
@jrose-apple jrose-apple deleted the command-p branch May 16, 2019 00:20
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants