Skip to content

Conversation

@jrose-apple
Copy link
Contributor

...instead of just ignoring the errors in certain cases, in service of source compatibility.

Swift 3.0 wasn't nearly as strict as checking access control for types because it didn't look at the TypeRepr at all (except to highlight a particular part of the type in diagnostics). It also looked through typealiases in certain cases. Approximate this behavior by running the access checking logic for Types (rather than TypeReprs), and downgrade access violation errors to warnings when the checks disagree.

Part of rdar://problem/29855782.

@jrose-apple
Copy link
Contributor Author

Part 1 of 2—a follow-up request will do something similar for availability. Credit to @slavapestov for the idea; I'm just the implementer.

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - ed89a62071e104d553c1bcf08f33e754e9d0575f
Test requested by - @jrose-apple

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - ed89a62071e104d553c1bcf08f33e754e9d0575f
Test requested by - @jrose-apple

@jrose-apple
Copy link
Contributor Author

Bad merge, but shouldn't affect review. Rebasing.

No functionality change; the value at the place where the callback is
invoked is always DowngradeToWarning::No. This will be exercised by
the next commit.
...instead of just ignoring the errors in certain cases, in service of
source compatibility.

Swift 3.0 wasn't nearly as strict as checking access control for types
because it didn't look at the TypeRepr at all (except to highlight a
particular part of the type in diagnostics). It also looked through
typealiases in certain cases. Approximate this behavior by running the
access checking logic for Types (rather than TypeReprs), and downgrade
access violation errors to warnings when the checks disagree.

Part of rdar://problem/29855782.
@jrose-apple jrose-apple force-pushed the access-reluctantly-granted branch from ed89a62 to 9489f10 Compare January 13, 2017 22:35
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - ed89a62071e104d553c1bcf08f33e754e9d0575f
Test requested by - @jrose-apple

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - ed89a62071e104d553c1bcf08f33e754e9d0575f
Test requested by - @jrose-apple

@slavapestov
Copy link
Contributor

Looks good!

highlightOffendingType(TC, diag, complainRepr);
// Swift 3.0.0 mistakenly didn't diagnose any issues when the context access
// scope represented a private or fileprivate level.
// FIXME: Conditionalize this on Swift 3 mode.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the fixme still apply?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately yes, this is a separate issue. It shouldn't be hard but the diagnostics need updating too. It's on my list.

@jrose-apple jrose-apple merged commit 8d164c5 into swiftlang:master Jan 15, 2017
@jrose-apple jrose-apple deleted the access-reluctantly-granted branch January 15, 2017 19:30
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