Re: [Async] Peer review report and conditions

From: Klemens Morgenstern (klemensdavidmorgenstern_at_[hidden])
Date: 2023-10-11 13:08:32

> 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!