1. The library needs to be renamed to something the Boost community doesn’t object to
Quite a bit of discussion and feedback surrounded the proposed name “Boost.Async”. Multiple people felt that the name claims too much seeing as it is a C++ 20 coroutines only library, and is from first appearances entirely tied into Boost.ASIO (technically, any ASIO i/o executor can wrap any third party executor, but I can see where people get the strong ASIO association).
I agree with Klemens that putting “ASIO” in the new name would be out of character with the other Boost libraries also heavily orientated around Boost.ASIO which do not have ASIO in their name.
I find the suggestions that C++ 20 coroutines needs to be somehow in the name convincing. Exactly what form it should take I’ll leave up to the author and community to negotiate, but “Boost.Async” is definitely not a name with good buy-in in the Boost community. It cannot be named “Boost.Async” if it is to enter Boost.
How do we judge what is acceptable to the boost community? Because there might always be someone to object to it. I'll compose a list and start a separate thread.
2. The `select()` operation either needs to have its semantics changed, or be renamed
Quite a few reviewers found the current semantics unpleasantly surprising, in that a random ready awaitable is chosen and all the others cancelled. This is far away from what `select()` typically means in the C and C++ worlds, and multiple reviewers commented negatively about this.
The docs are unclear if it is guaranteed that the first ready awaitable is always chosen, or whether it is _some_ ready awaitable. The difference is important in many kinds of async programming. As this is a single threaded implementation, it should be possible to offer strict guarantees here.
The semantics are acceptable if the operation is not called “select”, so I would suggest the easiest course here is to rename that operation to something more acceptable to the Boost community.
There is left_select, which goes strictly left to right. Would this criteria be satisfied if I renamed left_select to select, and the current select to `unordered_select` or `random_select` ?
- Andrzej took issue with the design of the generators, and I can see why. However, out in real world code land, C++ coroutine generators have surprisingly large amount of design variation. I’ve seen ones which you loop on testing them for boolean true, I’ve seen others which loop on their iterators, I’ve even seen ones which don’t implement either boolean true nor iterators and require you to call a `value()` member function which returns an Optional. Why? No idea, people seem to innovate uselessly here for some reason. I think it wise to adhere to `std::generator` from the standard, and do exactly what it does. From my reading of Async’s `generator`, whilst it has extensions e.g. the invocable call operator, the only thing missing is the iterator interface to make it implement `std::generator` and I don’t see any obvious reason why an iterator interface can’t be added here?
The reason is that we don't have "async for". I would need to resume the generator on increment, but if `itr++` is an awaitable expression that needs to be co_awaited, it's not an iterator. If there was an async for (or a common pattern) I'd make generator work with it.
My thanks to Klemens for putting the work into what is a controversial and opinionated library design quite unlike any other currently available. It’s not easy to tread new ground without much precedent.
My thanks to the Boost and Reddit communities for their ample and detailed review feedback, which made me managing this review possible.
Thank you for managing this double-period review!