-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Implement SE-0117 #3882
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
Implement SE-0117 #3882
Conversation
@swift-ci Please smoke test. |
@swift-ci Please smoke test |
(just restarting the build for John) |
@swift-ci Please test. |
@swift-ci Please smoke test. |
@swift-ci Please smoke test. |
Okay, just the same failure about xctest. @swift-ci Please test. |
That's a real failure, and a suggestive one. The new diagnostic is forcing a non-public method to be exposed just so the override can be open. We're creating an inconsistency here. After some discussion with Jordan and Doug, we're going to amend the proposal to lift that half of the "temporary" restriction, so open methods can override non-open ones. This shouldn't be a problem for hierarchy analysis, which has to exhaustively find overrides anyway and can easily recognize when one is open. |
So revised. @swift-ci Please test. |
The Linux failure is: fatal error: Attempt to invoke test on class TestNSAffineTransform with incompatible instance type XCTestCase: file /home/buildnode/jenkins/workspace/swift-PR-Linux@3/swift-corelibs-xctest/Sources/XCTest/Public/XCTestCase.swift, line 182 |
@swift-ci please smoke test Linux |
Yeah, I've been looking into it. This might be the first use of required inits; XCTest uses them heavily, and I'm worried we might be devirtualizing it (because it's not open) or something. |
Okay, that seemed to fix it. Trying to figure out a test case. In the meantime, @swift-ci Please test. |
One minor revision: this lifts the proposed restriction against overriding a non-open method with an open one. On reflection, that was inconsistent with the existing rule permitting non-public methods to be overridden with public ones. The restriction on subclassing a non-open class with an open class remains, and is in fact consistent with the existing access rule.
That last failure was a new test that hadn't been updated for 'open'. @swift-ci Please smoke test. |
I... don't think that's me? I don't know how to interpret that failure. |
Weird intermittent failure? @swift-ci Please smoke test. |
I should probably have renamed this pull request before actually merging it, huh. |
LOL. Congrats John! |
:) |
[pull] swiftwasm from main
What's in this pull request?
Resolved bug number: (SR-)
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.