SE-0388: Convenience Async[Throwing]Stream.makeStream methods

Hello Swift community,

The review of "Convenience Async[Throwing]Stream.makeStream methods" begins now and runs through February 26, 2023. The proposal is available here:

https://github.com/apple/swift-evolution/blob/main/proposals/0388-async-stream-factory.md

To assist in your review, I've prepared toolchains for macOS, Ubuntu 20.04, and Windows 10 which include the proposal's implementation.

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager by email. When emailing the review manager directly, please keep the proposal link at the top of the message and put "SE-0388" in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to Swift?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

https://github.com/apple/swift-evolution/blob/main/process.md

Happy reviewing,

Becca Royal-Gordon
SE-0388 Review Manager

24 Likes

+1 I have the following snippet in a code base I maintain and would love to remove it.

Extensions/AsyncStream+Init.swift
10:    public static func initEscapingContinuation(

I think using a type over a tuple makes sense, the only thing that strikes me as odd is the name "NewStream" as AsyncStream<T>.init() also produces a new stream.

Would this type/methods be marked as @_backDeployed so folks like myself can remove our versions of this api?

1 Like

Yes please! I find the current API extremely annoying in most cases (though there are some cases where it makes sense, itā€™s just not the majority of my usage). The code sample in the motivation section is a perfect example of the annoying issues I run into with this API.

I will also say that when I first came to this API I found it very unintuitive that the continuation was allowed to escape the closure, since I can think of no other ā€œmake a thing which is passed into a trailing closureā€ API in the standard library that allows escaping the ā€œthing passed in.ā€

On the alternatives considered:

  • I personally donā€™t care about returning a tuple or having a named type for it; both seem reasonable.
  • Strongly agree with the proposal that NewStream should not have an init (what a weird shape for the API that would be)
  • Strongly agree that AsyncStream.init should not take a continuation ā€” you shouldnā€™t be able to tie random continuations & streams together, and you shouldnā€™t be able to get a continuation that doesnā€™t go to any stream.

I did a light reading of the proposal, and have used these types before.

1 Like

Naming is hard :smiley: I went with the same name that we used in NIO. Open to better suggestions though!

SE-0376 only allows for functions to be back deployed. Since we are introducing a new type here we cannot back deploy this function. This is the big upside of using the tuple since it allows us to back deploy this.

Not sure this is actually a better suggestion, but my default would be to name it as it is on the tin: "StreamAndContinuation".

I definitely forgot about this restriction. Regardless, I do still think using a type is better in the long term and hoping for a solution to type back deployment later.

2 Likes

+1 That looks great! But I think the name NewStream sounds too generic. Maybe StreamWithContinuation or StreamContinuationPair? :thinking:

1 Like

Couldn't both versions be included with the one returning a tuple marked with backDeployed and _disfavoredOverload?

+1, this is a great addition, and sorely needed!

It definitely makes sense to add this to the stdlib - I see so many people doing the same boilerplate with the implicitly unwrapped AsyncStream<T>.Continuation!

I'm a big +0.9 on this. I think this fills an obvious gap in current APIs, but also I think we could potentially bikeshed on the name NewStream a little bit. Maybe WithContinuation? (ie AsyncStream<Element>.WithContinuation)

9 Likes

Any version of the functionality seems fine to me, however the alternative variant using a function could potentially be back ported for convenience via the @_alwaysEmitIntoClient (if it applies to static functions as well).

If weā€˜re bikeshedding, then AsyncStream<ā€¦>.detached could be another option as weā€˜re detaching the the streams continuation from the otherwise encapsulated build closure .

i've written and deleted such a type many times in the past and my conclusion has always been that it doesn't serve much purpose other than to glue two things together, which tuples do just as well.

the purpose of a type is to host API that wouldn't make sense to place anywhere else. the proposed NewStream type doesnā€™t have any API.

perhaps it would be useful to have NewStream if it had an init that could be accessed with leading dot syntax. but the proposal seems to have explicitly ruled that out. so as it stands i feel that the new type serves no purpose.

i think a tuple return is better for this API.

9 Likes

+1 - allows us to drop a helper.

Our internal solution for this uses a tuple, but as was pointed out during the pitch a type allows for better documentation and discoverability which is nice (which would make me support the type). (Regardless, would be +1 for the tuple variation too, it works fine for us now)

Iā€™d agree with the suggestion for bikeshedding the naming a bit - WithContinuation as suggested by @JuneBash probably the best suggestion so far.

1 Like

+1 Overall.

Naming is definitely awkward though.

I would say I prefer the tuple over introducing a new type. I think the documentation argument is negated by the simplicity of the type.

AsyncStream, AsyncStream.Continuation, and makeStream can all be documented so not sure what else is needed here. Seems crystal clear.

It also seems to be the API a bunch of people have come up with completely independently and organically which suggests itā€™s a natural fit.

If there must be a type, I think StreamContinuationPair says all it needs to.

7 Likes

+1
No brainer, should have been included from the start, but we all work iteratively :)

+1. I don't really see the need for a type here, though, unless we expect the NewStream type to gain additional API over time ā€” which I don't think we do? A tuple with labels seems perfectly suited for this use case. I don't find the documentation argument holds much weight; the documentation comments shown in the code snippets are short enough they can happily live as part of the makeStream method.

(The back-deployability of the tuple approach is an added bonus.)

8 Likes

This feature addresses a strong need. Thank you for addressing it.

Reading the example, Iā€™d changeā€¦

for await i in newStream.stream {
      print(i)
    } 

ā€¦Toā€¦

 for await i in newStream.values {
      print(i)
    }

Or maybe newStream.elements or newStream.items.

I often hear this paradigm as ā€œvalues over timeā€.

Definitely needed.

I would suggest a name like AsyncStream.pipe(). Returning either a tuple, or a type AsyncStreamPipe.

Pipe fits well with the idea of connecting an input (the continuation), to an output (the stream per se), Plus this already is a word used in the Unix world right?

withoutSpeakingAsReviewManager {

I haven't decided where I come down on the result type question, but I'd like to point out a third alternative: make NewStream a typealias for a tuple. This would have the back-deployment* and destructuring advantages of a tuple, but would still allow us to attach documentation to the typealias (just not the individual properties).

* Unlike structs and other nominal types, typealiases are resolved at compile time and have no ABI, so they don't prevent back-deployment.

}
18 Likes

Looks like a great addition to the standard library.

The NewStream name seems a little redundant (AsyncStream.NewStream) and yet insufficient. I think something like Endpoints would work better, as the object contains both the read endpoint (the AsyncStream) and the write endpoint (the Continuation).

+1 in general, but I would prefer a tuple over a type. I already use a similar helper in my own projects which uses a tuple with labels and that works great. I don't buy the documentation argument and a new type also comes with downsides like not being able to backdeploy.


make NewStream a typealias for a tuple.

I personally don't think a typealias is even needed. The tuple elements can just be documented on the makeStream() method.

4 Likes