Re: [boost.async] a note on documentation

From: Klemens Morgenstern (klemensdavidmorgenstern_at_[hidden])
Date: 2023-09-23 13:33:18

On Sat, Sep 23, 2023 at 6:55 PM Andrzej Krzemienski <akrzemi1_at_[hidden]> wrote:
>
>
>
> pt., 22 wrz 2023 o 17:02 Klemens Morgenstern <klemensdavidmorgenstern_at_[hidden]> napisał(a):
>>
>> On Fri, Sep 22, 2023 at 10:34 PM Andrzej Krzemienski via Boost
>> <boost_at_[hidden]> wrote:
>> >
>> > Hi Everyone,
>> > In the context of the Boost review of Boost.Async, I wanted to share a
>> > thought on documentation.
>> > I think the Reference section in the docs is insufficient. A lot of
>> > libraries in Boost have this approach that the reference section of the
>> > documentation is a specification exhaustive enough that it could be used to
>> > provide a number of competing implementations. Each function has a very
>> > detailed contract: what it returns and when, what are the preconditions,
>> > what exceptions are thrown upon failure, what are the postconditions. I am
>> > missing this from the reference of Boost.Async.
>>
>> Post-conditions are not really a good fit for asynchronous code I
>> fear, especially since a user can omit a co_await.
>> You'd end up with a confusing mess IMO.
>
>
> I agree with this statement. (BTW, we have an entire paper on this: wg21.link/P2957)
>
> Yet, I still maintain that this documentation is lacking a description of what it expects and what it guarantees, even if you cannot call it "a postcondition on the function". Consider the specs for `select` as an example:

Well I won't argue that my docs are a bit brief. But I try to keep
them as brief as possible to minimize noise. I hope you noticed that I
have indeed reacted to much of your feedback when improving the docs.

I am also not planning on stopping writing the docs either, IMO docs
are always a WIP.

>
> https://klemens.dev/async/reference.html#select
>
> After reading it I am not sure I know what select() does, especially in the corner cases. I suppose that if I were familiar with other async frameworks from node.js or python I would know the answer.

Not really, select only exists in golang as a language construct.

>
> I am sure you want to give me a *guarantee* of some sort; maybe not on the call to select alone, but on the expression `co_await select(args...)`. So, what is it?
> The effect would be as if I called co_await on one (but it is not specified which) of the awaitables from `args...`. Did I guess that right?

Did you look at the design section? I didn't want to clutter the
reference with this:
https://klemens.dev/async/design.html#design:select

>
> What happens when I call `co_await select(args...)` and all the awaitables in `args...` have already been co_awaited on?
>

Depends on the awaitables. Whatever they do happens.

> What happens if I pass zero arguments to `co_await select(args...)`?

Compile error (also one for one argument that's not a range).

>
> What happens if I pass an empty vector to `co_await select(args...)`?

Exception (that's in the docs).

>
> How do the results change if I pass a random number generator?

Undefined, the evaluation order changes. See below.

> How does the state of non-selected awaitables change after the call to `co_await select(args...)`?

Undefined. They might be awaited & interrupted or cancelled, or they
might not be touched at all. That's undefined on purpose and depends
on the random number generator.

Let's say you do a `select(a, b, c)` and a is ready (await_ready
returns true), while b & c are not.
If the random order is (a, b, c), then b & c will not get touched. If
the order is (b, a, c), then b will get awaited &
interrupted/cancelled, while c is untouched.
If it's (b, c, a), then b & c will get awaited & interrupted and cancelled.

> If the answer to any of these questions is "you mustn't do that", this should be listed as a precondition.

Well the `select` is designed to work with any awaitable, just like
everything else in async.
So it will await a random (i.e. undefined) subset of the awaitables
passed in and return the first (i.e. undefined) to complete,
and then will either interrupt (if the awaitable supports it, i.e.
undefined) or cancel and disregard the result.

Three things here are undefined by design, so it's hard to give strict
criteria I find.

>
>
>> But I might add, that I don't
>> like to read this kind of documentation either.
>>
>> >
>> > Also, it may be one of the first libraries (that meet a certain bar of
>> > documentation) tied so much to coroutines, so I have no strict requirements
>> > on how a coroutine library is documented, but I think things like promise
>> > types should be explicitly referenced.
>>
>> I did this by having base types for each promise which are indeed documented
>>
>> https://klemens.dev/async/reference.html#generator-promise
>
>
> What I am missing is a section in Design part of documentation that says that a number of features are implemented as base classes that promise-types are expected to derive from.

Fair enough, that's currently in the reference:
https://klemens.dev/async/reference.html#concepts

>
> For a feature like "cancellation" or "cancellation state" I would expect a synopsis of class `promise_throw_if_cancelled_base` along with its function `await_transform` and the description of what the function does.

I get that request, but I don't know if that will help users. I know
of more than one asio user who have used `co_await
this_coro::executor` without ever knowing what `await_transform` is.
In my opinion, await_transform just provides a pseudo-awaitable
(https://klemens.dev/async/reference.html#this_coro) that are valid if
a coroutine type opts-in through inheritance.
`await_transform` is an implementation detail.

>
>>
>>
>> >
>> > Let's consider `async::generator`. It is not only class template
>> > `generator`, but also:
>> >
>> > 1. the specialization of type trait std::coroutine_traits
>>
>> It doesn't have that specialization.
>
>
> My bad.
> But it has a public alias promise_type which has an effect on what guarantees I get.
>

Well that needs to be public to work with the C++ api. I am not
considering this part of the public interface though, just like
`detail` namespaces technically are public.

>> >
>> > Even though some of them belong to namespace `detail`, they provide
>> > guarantees relevant for the users. I do not think `generator_promise`is an
>> > implementation detail. I think it needs to be documented. This seems
>> > especially important when I start adding my awaitables to the mix.
>>
>> It shouldn't be. The recommended way is to check for associators
>> through concepts, as describe here:
>>
>> https://klemens.dev/async/design.html#associators
>
>
> I do not know what to make of this. I am not well familiar with associators yet. The docs says "async uses the associator concept of asio, but simplifies it." I read it as saying that one cannot add one's own awaitables until one understands the associator concept of ASIO.
>

Not really, there's a code-snippet showing all there is to it (and you
don't even need them in many cases).

> Still I am convinced that as a user of this library I need to know what is going on in `await_transform` of different types, and when I am engaging them.

You don't: https://klemens.dev/async/reference.html#enable_awaitables

>
> I hope this makes sense. I have little experience with coroutines, so I do not know which of the choices applied in Boost.Async are a necessary part of every C++ coroutine library, and which are unique choices specific to this one.

That does make sense. I am taking a lot of knowledge from other
languages for granted for sure.

>
> Regards,
> &rzej;
>>
>>
>>
>> >
>> > I wonder what others think about it?
>> >
>> > Regards,
>> > &rzej;
>> >
>> > _______________________________________________
>> > Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost