Check protocol witnesses using access scopes. #4176
Conversation
|
@swift-ci Please test |
|
@slavapestov or @CodaFi, mind reviewing this? |
|
Oops, broke a crash case (in addition to the phantom copyforward issue that all the bots are seeing). |
...rather than relying on the access-as-spelled, which may be greater than the effective access due to parent scopes. (Some of this will get cleaned up with SR-2209.) rdar://problem/27663492
jrose-apple
force-pushed
the
check-conformance-using-access-scopes
branch
from
8d60995 to
9b375d3
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Linux platform |
|
@swift-ci Please smoke test OS X platform |
|
@swift-ci Please test Linux platform |
|
@swift-ci Please smoke test OS X platform |
1 similar comment
|
@swift-ci Please smoke test OS X platform |
|
@swift-ci Please test Linux platform |
|
Okay, this got all the way to Foundation tests on Linux, merging. |
|
Review ping for @slavapestov or @rjmccall? |
| if (isa<ModuleDecl>(accessScope)) | ||
| return Accessibility::Internal; | ||
| if (accessScope->isModuleScopeContext() && | ||
| accessScope->getASTContext().LangOpts.EnableSwift3Private) { |
There was a problem hiding this comment.
Do we still need this staging flag?
There was a problem hiding this comment.
Probably not, but I'm going to stay consistent with it until we take it out completely.
|
Looks good. |
| // that a broader scope is acceptable breaks some diagnostics. | ||
| if (attr->getAccess() != desiredAccess) { | ||
| // This uses getLocation() instead of getRange() because we don't want to | ||
| // replace the "(set)" part of a setter attribute. |
There was a problem hiding this comment.
Is there a test for this?
There was a problem hiding this comment.
Some of the existing tests failed when I tried to check for a narrower scope instead of a different scope, but I should probably add one for the "broader than desired" case in the FIXME. Thanks for the reminder.
There was a problem hiding this comment.
I just meant tests for the (set) part.
There was a problem hiding this comment.
Oh, yes, there are existing tests for that. That hasn't changed.
...rather than relying on the access-as-spelled, which may be greater than the effective access due to parent scopes. (Some of this will get cleaned up with SR-2209.) rdar://problem/27663492 (cherry picked from commit f65ad81)
...rather than relying on the access-as-spelled, which may be greater than the effective access due to parent scopes.
(Some of this will get cleaned up with SR-2209.)
rdar://problem/27663492
Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
A smoke test on macOS does the following:
device standard libraries are not built.
version of these tests are not run.
A smoke test on Linux does the following:
tests are not run.
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.