-
Notifications
You must be signed in to change notification settings - Fork 51
Migrate swiftly to Swift 6 with concurrency checks #310
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
Conversation
@swift-ci test macOS |
…t6_concurrency
@swift-ci test macOS |
@@ -399,11 +403,12 @@ struct StableReleaseParser: ToolchainSelectorParser { | |||
/// - swift-a.b-DEVELOPMENT-SNAPSHOT-YYYY-mm-dd | |||
/// - swift-a.b-DEVELOPMENT-SNAPSHOT | |||
struct ReleaseSnapshotParser: ToolchainSelectorParser { | |||
static let regex: Regex<(Substring, Substring, Substring, Substring?)> = | |||
static func regex() -> Regex<(Substring, Substring, Substring, Substring?)> { |
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.
What is the motivation to making this a function? We'll incur the regex creation cost every call. Could this be a lazy static instead?
static let regex: Regex = {
return try! Regex("^(?:swift-)?([0-9]+)\\.([0-9]+)-(?:snapshot|DEVELOPMENT-SNAPSHOT|SNAPSHOT)(?:-([0-9]{4}-[0-9]{2}-[0-9]{2}))?(?:-a)?$")
}
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.
No, it's because Regex is not Sendable in its current state. A lot of swift code just constructs regexes where they are used, so this would be no different. Maybe someday Regex can be Sendable.
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.
There's a forum thread on this topic:
https://forums.swift.org/t/should-regex-be-sendable/69529/15
@@ -423,7 +423,7 @@ public enum SwiftlyTests { | |||
} | |||
} | |||
|
|||
public class TestOutputHandler: SwiftlyCore.OutputHandler { | |||
public actor TestOutputHandler: SwiftlyCore.OutputHandler { | |||
public var lines: [String] |
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.
nit:
public var lines: [String] | |
public private(set) var lines: [String] |
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.
Fixed, thanks.
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.
Done
No description provided.