Rework AsyncStreaming protocols to use generic `RangeReplaceableCon…#417
Conversation
Catfish-Man
left a comment
Catfish-Man
left a comment
There was a problem hiding this comment.
A few minor comments, but basically looks great. Thanks for doing this!
| upTo limit: Int, | ||
| body: (consuming InputSpan<ReadElement>) async throws(Failure) -> Result | ||
| ) async throws(EitherError<ReadFailure, Failure>) -> Result { | ||
| ) async throws(EitherError<EitherError<ReadFailure, AsyncReaderLeftOverElementsError>, Failure>) -> Result { |
There was a problem hiding this comment.
EitherError<EitherError<…>> wow that's a painful type. Do we need a variadic error instead?
There was a problem hiding this comment.
I don't think you can define a variadic enum that generates cases per pack element. I agree this is awful...
| maximumCount: limit - buffer.count | ||
| ) { (span: consuming InputSpan<ReadElement>) in | ||
| guard span.count > 0 else { | ||
| try await self.read { (buffer: inout Buffer) throws(AsyncReaderLeftOverElementsError) -> Void in |
There was a problem hiding this comment.
My kingdom for better typed throws inference in closures
There was a problem hiding this comment.
IIRC the compiler can do this but we disabled it since it is source breaking. We really should have an upcoming feature that infers this by default again
|
|
||
| > If you are not sure, pick callee-owned (`AsyncReader`) for read streams and | ||
| > caller-owned (`AsyncWriter`) for write streams. | ||
| > caller-owned (`CallerAsyncWriter`) for write streams. |
There was a problem hiding this comment.
Hm. Weren't we going to give the "good" names to the recommended types?
There was a problem hiding this comment.
Yes, this is still like the current state. I wanna revisit the names altogether. I have some better ideas potentially but for now keeping the two sides aligned is easier to think about.
| extension AsyncReader where Self: ~Copyable, Self: ~Escapable { | ||
| /// Iterates over all chunks, executing `body` for each buffer until the | ||
| /// stream ends. | ||
| public consuming func forEachBuffer<Failure: Error>( |
There was a problem hiding this comment.
Do we want to have the proposal reflect the current state or speculate about the possible syntax sugar?
There was a problem hiding this comment.
My intention was the current state for now and keep it updated with changes.
| @Test | ||
| @available(macOS 26.2, iOS 26.2, watchOS 26.2, tvOS 26.2, visionOS 26.2, *) | ||
| func collectIntoOutputSpan() async { | ||
| // TODO: Cannot test this yet since we can't get `InputSpan`s available in async contexts |
FranzBusch
force-pushed
the
fb-async-streaming-container
branch
from
adaf220 to
e739f9b
Compare
…tainer` buffers Replace `InputSpan`/`OutputSpan` with generic `Buffer` associated types constrained to `RangeReplaceableContainer` across all four streaming protocols. This sidesteps the limitation that `OutputSpan` (and other `~Escapable` types) cannot be used in async contexts today, while still allowing conforming types to choose a buffer representation optimized for their use case (e.g. `UniqueArray` for heap-backed storage, or a future stack-allocated container for embedded). Switch callee-owned protocols from `consuming` to `inout` closure semantics, rename `forEachChunk` to `forEachBuffer`, and move `forEach`/`collect` to future directions in the proposal. Adds test coverage for `AsyncWriter`, `CallerAsyncReader`, and `EitherError`. Updates all documentation to reflect the new API surface.
FranzBusch
force-pushed
the
fb-async-streaming-container
branch
from
e739f9b to
f7a915e
Compare
2 participants
…tainer` buffers
Replace
InputSpan/OutputSpanwith genericBufferassociated types constrained toRangeReplaceableContaineracross all four streaming protocols. This sidesteps the limitation thatOutputSpan(and other~Escapabletypes) cannot be used in async contexts today, while still allowing conforming types to choose a buffer representation optimized for their use case (e.g.UniqueArrayfor heap-backed storage, or a future stack-allocated container for embedded).Switch callee-owned protocols from
consumingtoinoutclosure semantics, renameforEachChunktoforEachBuffer, and moveforEach/collectto future directions in the proposal.Adds test coverage for
AsyncWriter,CallerAsyncReader, andEitherError. Updates all documentation to reflect the new API surface.