Skip to content

Remove @unchecked Sendable conformance from ChannelOptions.Storage#2638

Merged
glbrntt merged 3 commits intoapple:mainfrom
FranzBusch:fb-channel-option-sendable
Feb 12, 2024
Merged

Remove @unchecked Sendable conformance from ChannelOptions.Storage#2638
glbrntt merged 3 commits intoapple:mainfrom
FranzBusch:fb-channel-option-sendable

Conversation

@FranzBusch
Copy link
Copy Markdown
Member

Motivation

We currently have an unchecked Sendable conformance on ChannelOptions.Storage which is not necessary and in fact we are missing a proper Sendable requirement on the ChannelOption.Value protocol right now.

Modification

This PR adds the Sendable requirement on the ChannelOption.Value and replaces the unchecked conformance on the storage with a regular one.

Result

No more Sendable warnings in strict concurrency mode inside ChannelOptions.

@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Feb 1, 2024
@FranzBusch
Copy link
Copy Markdown
Member Author

Breaking change is a false positive:

💔 API breakage: protocol ChannelOption has generic signature change from <Self : NIOCore._NIOPreconcurrencySendable, Self : Swift.Equatable> to <Self : NIOCore._NIOPreconcurrencySendable, Self : Swift.Equatable, Self.Value : Swift.Sendable>

@FranzBusch
Copy link
Copy Markdown
Member Author

@swift-server-bot test this please

# Motivation
We currently have an unchecked `Sendable` conformance on `ChannelOptions.Storage` which is not necessary and in fact we are missing a proper `Sendable` requirement on the `ChannelOption.Value` protocol right now.

# Modification
This PR adds the `Sendable` requirement on the `ChannelOption.Value` and replaces the unchecked conformance on the storage with a regular one.

# Result
No more `Sendable` warnings in strict concurrency mode inside `ChannelOptions`.
@FranzBusch FranzBusch force-pushed the fb-channel-option-sendable branch from cdd77e9 to 010e6f3 Compare February 2, 2024 13:23
@FranzBusch FranzBusch force-pushed the fb-channel-option-sendable branch from 5bcd73c to 31457e6 Compare February 9, 2024 13:42
@FranzBusch
Copy link
Copy Markdown
Member Author

@swift-server-bot test this please

@FranzBusch FranzBusch force-pushed the fb-channel-option-sendable branch 2 times, most recently from ae61486 to 9fc8804 Compare February 9, 2024 16:50
@FranzBusch FranzBusch force-pushed the fb-channel-option-sendable branch from 9fc8804 to 72ae400 Compare February 11, 2024 12:23
@FranzBusch
Copy link
Copy Markdown
Member Author

The breaking change is expected as above. Sendable is only producing warnings before Swift 6 mode. The nightly CI is running into a compiler crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔨 semver/patch No public API change.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants