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
Bump compiler version to Swift 5. #20374
Conversation
Main pieces: - Bump swift-version to 5 - Remove -swift-version 3 support
@swift-ci test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good!
@@ -37,7 +37,7 @@ struct S { | |||
} | |||
|
|||
enum E { | |||
case NoElement() | |||
case NoElement(Void) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: this one should probably go to case NoElement
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jrose-apple, but I don't quite understand. This change is needed or else the test fails. The compiler now complains about the missing Void
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unclear whether the test is supposed to be testing a case with no payload or a case with a zero-sized payload, but given the name of the enum case I'm guessing it's the former.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's the latter. @CodaFi banned enums with an empty tuple payload so that we can stage in the 'normalize enum cases' proposal later without breaking source compatibility, and I recall discussing the intent behind this test case at the time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you for correcting the migrator test.
Build failed |
Build failed |
@swift-ci test macOS |
Build failed |
# Conflicts: # validation-test/stdlib/HashedCollectionFilter3.swift # validation-test/stdlib/HashingPrototype.swift
@swift-ci clean test |
Build failed |
Build failed |
Failure building Corelibs-Foundation:
|
Corelibs issue: |
# Conflicts: # test/api-digester/Outputs/stability-stdlib-source.swift.expected
@swift-ci test |
Build failed |
Build failed |
@swift-ci test |
@swift-ci test |
Build failed |
Build failed |
@swift-ci clean test |
Build failed |
@swift-ci test linux |
Build failed |
@swift-ci test linux |
Build failed |
Build failed |
@swift-ci smoke test |
1 similar comment
@swift-ci smoke test |
Waiting on: |
Please test with apple/swift-corelibs-foundation#1768 @swift-ci smoke test |
Please test with apple/swift-corelibs-foundation#1768 @swift-ci smoke test linux |
@swift-ci smoke test linux |
please test with apple/swift-corelibs-xctest#237 @swift-ci smoke test linux |
@swift-ci smoke test linux |
please test with apple/swift-corelibs-xctest#238 @swift-ci smoke test linux |
please test with apple/swift-corelibs-foundation#1774 @swift-ci smoke test linux |
please test with apple/swift-corelibs-foundation#1775 @swift-ci smoke test linux |
1 similar comment
please test with apple/swift-corelibs-foundation#1775 @swift-ci smoke test linux |
@swift-ci smoke test and merge |
Historically, this test was verifying some reflection support that was used for Playgrounds support. Today, through API changes, this was just testing that we could create an print a specific enum. With the switch to Swift 5.0 (apple/swift#20374), this enum is now deprecated and the test fails. As the coverage provided here seems pretty insignificant, let's just remove the test.
Main pieces:
Subsumes #18853