SE-0298: Async/Await: Sequences

[xwu (Xiaodi Wu)]

@Douglas_Gregor

• 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?

This is an important building block for Swift's concurrency story. The problem is significant enough to warrant such an addition, and in general it fits well with the feel and direction of Swift. I do not have hands-on experience with other languages with similar features, but I have followed this and the related pitches closely and I studied the final proposal in some depth.

I do have a few comments which I really do hope will be considered before this proposal is accepted:

cancel()

If next() returns nil then the iteration ends naturally and the compiler does not insert a call to cancel() . If next() throws an error, then iteration also ends and the compiler does not insert a call to cancel() . In both of these cases, it was the AsyncSequence itself which decided to end iteration and there is no need to tell it to cancel.

If the AsyncIterator is a class type, it should assume that deinit is equivalent to calling cancel . This will prevent leaking of resources in cases where the iterator is used manually and cancel is not called.

I am not sure how to square these two points conceptually. On the one hand, cancel() is not called one we're done with iteration, and users must understand that what they put inside cancel() won't be executed in that circumstance. But on the other hand, if the iterator is a class, then users can rely on cancel() being equivalent to deinit?

If @Chris_Lattner3 is correct that we don't technically need this API to enable any designs, and future language evolution (introduction of deinit for structs) will define away its need completely, then I think it makes sense to strike cancel() from this proposal. I do fear that it will be quite confusing to users about how much "finality" there is in cancel() versus deinit.

AsyncSequence.first()

I do very much agree that consistency in the API design of Sequence and AsyncSequence is a laudable goal, and it is achieved quite well for the most part. However, if accepted as proposed, first() would be a function here whereas it is a property on Collection. This may lead users to be confused as to why there is such a difference, and overall it creates an inconsistency in our API naming. Indeed, the proposal authors here write that the reason for the difference is that "properties cannot throw"--but, indeed, another reason is that they also cannot be async (as per the core team's decision in SE-0296).

As it happens, we don't need to have throwing and async getters in order to resolve this inconsistency. Notably, Sequence doesn't have the property first, only first(where:), and by the same token it's fine if it also doesn't exist on AsyncSequence. One can always add this property later (to both Sequence and AsyncSequence) if it's deemed useful enough, once it becomes possible to do so without contorting ourselves in terms of API design.