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

Fully adopt Sendable #151

Merged
merged 5 commits into from Jul 4, 2023
Merged

Fully adopt Sendable #151

merged 5 commits into from Jul 4, 2023

Conversation

dnadoba
Copy link
Member

@dnadoba dnadoba commented Jun 30, 2023

No description provided.

@dnadoba dnadoba added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Jun 30, 2023
Comment on lines +65 to 72
let banner: SSHServerConfiguration.UserAuthBanner?
if case .server(let config) = role {
banner = config.banner
} else {
banner = nil
}

return .possibleFutureMessage(future.map { Self.transform($0, banner: banner) })
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes a warning with strict concurrency checking enabled

@dnadoba
Copy link
Member Author

dnadoba commented Jun 30, 2023

@swift-server-bot test this please

@Lukasa
Copy link
Collaborator

Lukasa commented Jul 3, 2023

@swift-server-bot test this please

@dnadoba
Copy link
Member Author

dnadoba commented Jul 4, 2023

Allocation counters regressions are unrelated to this PR:
#152

@dnadoba
Copy link
Member Author

dnadoba commented Jul 4, 2023

Allocation have regressed because I marked the static function transform(:_) with @Sendable to workaround this issue: apple/swift#61449

However, we can also just form a closure manually and don't annotate transform(_:) with @Sendable which doesn't produce Sendable warnings and doesn't allocate.

This bug appears to be already fixed in the latest swift nightly.

@dnadoba dnadoba merged commit d7279ea into apple:main Jul 4, 2023
8 checks passed
@dnadoba dnadoba deleted the dn-sendable branch July 4, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants