Re: [boost.async] a note on documentation

From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2023-09-23 14:17:01

sob., 23 wrz 2023 o 15:34 Klemens Morgenstern <
klemensdavidmorgenstern_at_[hidden]> napisał(a):

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

Indeed. And I appreciate it.

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

Thanks. That gives a good overview.
I would expect to find everything relevant to how to use `select` under the
reference section for `select`. This is how I understood the purpose of
"reference" sections for my entire career as a software developer. It is a
long section that you do not learn initially. But later, when you need to
look up (as in the dictionary or encyclopaedia) what the function is and
does, you go to "reference".

>
>
> >
> > 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).
>

I do not see it in https://klemens.dev/async/reference.html#select

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

I would hope to find information like this in the reference section.
Leaving some stuff intentionally as unspecified is a good thing.
But again, I would expect a clear indication of what is being left
unspecified intentionally.

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

That section says:  Inheriting enable_awaitables will enable a coroutine to
co_await anything through await_transform that would be co_await-able in
the absence of any await_transform.

Under my present understanding of the library, it means that
enable_awaitables does nothing: if things I pass are co_awaitable anyway,
why would I enable anything?

Regards,
&rzej;

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