-
Notifications
You must be signed in to change notification settings - Fork 113
adoption of sendable #252
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
adoption of sendable #252
Conversation
2b236e4
to
17051a1
Compare
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 think the really important type is LambdaRuntime
, which we need to make Sendable
.
c80f7c7
to
28d613b
Compare
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.
Great progress! I think we must not forget to mark LambdaRuntime
and LambdaTerminator
also as Sendable
.
yes, I will merge the terminator PR first than rebase and update |
motivation: adopt to sendable requirments in swift 5.6 changes: * define sendable shims for protocols and structs that may be used in async context * adjust tests * add a test to make sure no warning are emittted
@@ -137,3 +137,10 @@ extension LambdaTerminator { | |||
let underlying: [Error] | |||
} | |||
} | |||
|
|||
// Ideally this would not be @unchecked Sendable, but Sendable checks do not understand locks | |||
// We can transition this to an actor once we drop support for older Swift versions |
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.
We can transition this to an actor once we have custom executors :)
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.
that too
done for LambdaTerminator. why would we want LambdaRuntime to be Sendable? it is not used directly by the users atm afaik. |
@tomerd LambdaRuntime should be
After that we should have marked everything in the public API as Sendable that needs to 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.
Awesome! I think we can make one comment a little clearer. Besides that, great work!
Co-authored-by: Fabian Fett <fabianfett@apple.com>
motivation: adopt to sendable requirments in swift 5.6
changes: