-
Notifications
You must be signed in to change notification settings - Fork 634
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
Adopt Sendable
in EventLoop.swift
#2101
Conversation
Sources/NIOCore/EventLoop.swift
Outdated
@@ -19,15 +19,28 @@ import Dispatch | |||
/// | |||
/// A `Scheduled` allows the user to either `cancel()` the execution of the scheduled task (if possible) or obtain a reference to the `EventLoopFuture` that | |||
/// will be notified once the execution is complete. | |||
public struct Scheduled<T> { | |||
public struct Scheduled<T>: NIOSendable { |
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.
Does this need to be preconcurrency
? We can only be Sendable
where the generic type is 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.
Good question and I think the question boils down to if EventLoopPromise
should be unconditionality Sendable
or not.
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 have made it only conditionally Sendable
as EventLoopPromise
should have been only conditionally Sendable
as well.
Sources/NIOCore/EventLoop.swift
Outdated
#endif | ||
|
||
#if swift(>=5.6) | ||
@preconcurrency |
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.
Does @preconcurrency
do anything for us if this is internal
only?
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 isn't necessary here, removed.
Sources/NIOCore/EventLoop.swift
Outdated
/* private but usableFromInline */ @usableFromInline let _cancellationTask: () -> Void | ||
#endif | ||
|
||
#if swift(>=5.6) |
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.
Should this be 5.6 or 5.7?
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.
Update all to 5.7
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.
Nice stuff, thanks @dnadoba.
API breakage change is expected. |
No description provided.