adds AsyncChannel and AsyncThrowingChannel with tests #2086
Conversation
|
|
||
| import Foundation | ||
|
|
||
| public actor AsyncChannel<Element: Sendable>: AsyncSequence { |
There was a problem hiding this comment.
Do we need public keyword for all these?
There was a problem hiding this comment.
As discussed lets make these internal
There was a problem hiding this comment.
done, though the AsyncSequence is a protocol with associated type. There is no way to represent it in Swift for a return value without the implementation
| } | ||
|
|
||
| mutating func next() async -> Element? { | ||
| await channel.next() |
There was a problem hiding this comment.
Check for cancellation? https://developer.apple.com/documentation/swift/asynciteratorprotocol
There was a problem hiding this comment.
What can be canceled here?
| init() { | ||
| } | ||
|
|
||
| nonisolated func makeAsyncIterator() -> Iterator { |
There was a problem hiding this comment.
Isnonisolated okay? Will the channel work if we iterate in two different task?
There was a problem hiding this comment.
this function cannot be isolated in the actor. it also does not mutate any state. It creates the iterator which is used by the sequence and all those functions are isolated within the actor.
| } | ||
|
|
||
| private var canTerminate: Bool { | ||
| terminated && elements.isEmpty && !continuations.isEmpty |
There was a problem hiding this comment.
this logic is correct
|
|
||
| func finish() { | ||
| terminated = true | ||
| processNext() |
There was a problem hiding this comment.
why are we calling processNext after finishing?
There was a problem hiding this comment.
see the processNext function
|
|
||
| import Foundation | ||
|
|
||
| actor AsyncChannel<Element: Sendable>: AsyncSequence { |
There was a problem hiding this comment.
nit: Call it AmplifyAsyncChannel ?
There was a problem hiding this comment.
there is reason to specialize this name
There was a problem hiding this comment.
I think if developer uses both Amplify and AsyncAlgorithms this names can conflict. We cannot use module name to distinguish between them because of the known issue in SPM where it cannot figure out the type Amplify vs the module Amplify. swiftlang/swift#43510
Issue #, if available:
Description of changes:
adds AsyncChannel and AsyncThrowingChannel with tests
Check points: (check or cross out if not relevant)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.