Skip to content
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

Conform EventLoopFuture/Promise to Sendable #1982

Merged
merged 3 commits into from
Nov 1, 2021

Conversation

adam-fowler
Copy link
Contributor

Conform EventLoopFuture and EventLoopPromise to Sendable

Motivation:

Fixes compile warnings

Based off discussion in #1928

@fabianfett
Copy link
Member

I'm afraid we will need to do this dance to make 5.2 - 5.4 happy:

#if swift(>=5.5)
public typealias NIOSendable = Swift.Sendable
#else 
public typealias NIOSendable = Any
#endif

Then NIO types need to conform to NIOSendable instead of Sendable.

@adam-fowler
Copy link
Contributor Author

I'm afraid we will need to do this dance to make 5.2 - 5.4 happy:

#if swift(>=5.5)
public typealias NIOSendable = Swift.Sendable
#else 
public typealias NIOSendable = Any
#endif

Then NIO types need to conform to NIOSendable instead of Sendable.

Yeah forgot about that. I have duplicated the code from the original PR, which adds the conformance in a separate extension but yeah maybe implementing NIOSendable is the way to go as in the future there will be a ton of these.

@Lukasa
Copy link
Contributor

Lukasa commented Oct 29, 2021

I think we should do NIOSendable. Better to do it right the first time.

@adam-fowler
Copy link
Contributor Author

We'll still have to stick #if swift(>=5.5) around the EventLoopFuture extension as @unchecked is only available in swift 5.5.
I'll add the typealias still. Do you want it in AsyncAwaitSupport.swift?

@Lukasa
Copy link
Contributor

Lukasa commented Oct 29, 2021

Nah, it'll have to go into NIOCore.

@adam-fowler
Copy link
Contributor Author

Nah, it'll have to go into NIOCore.

AsyncAwaitSupport.swift is in NIOCore

Copy link
Contributor

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Lukasa Lukasa added the semver/minor Adds new public API. label Nov 1, 2021
@Lukasa Lukasa merged commit 6975036 into apple:main Nov 1, 2021
@adam-fowler adam-fowler deleted the sendable-eventloopfuture branch November 1, 2021 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants