Re: [Async] Review of proposed Boost.Async begins

From: Klemens Morgenstern (klemensdavidmorgenstern_at_[hidden])
Date: 2023-08-18 00:24:47

On Fri, Aug 18, 2023 at 5:59 AM Ruben Perez via Boost
<boost_at_[hidden]> wrote:
>
> Hi all,
>
> This is my review of the proposed Boost.Async.

Thank you so much for taking the time to review my library!

>
> What's the rationale behind BOOST_ASYNC_USE_STD_PMR and friends? Being a
> C++20 library, why is std::pmr not enough? From the Jamfile, the library is
> built only with std::pmr by default. This looks like a source of problems -
> you need to explain to users how to build your library with Boost.Container pmr
> and without it. Config macros like these increase maintenance effort a lot.

That's because clang only added pmr support in 16, but had decent
coroutine support for
a few versions. Additionally some people feel clang's quality went
downhill, so they're stuck on clang-14.

>
> I've also noticed that boost::async::this_coro::executor_t is an alias for
> boost::asio::this_coro::executor_t. I think this is confusing - if
> I got the implementation right, no functionality from Asio is used here,
> but just the name. Can we not define an async::executor_t?
>

It can be done rather easily.
I thought it didn't hurt, and accidentally using
asio::this_coro::executor would still work.
These types are more like pseudo-keywords, so I didn't want to make
them restrictive.

>
> - Please explicitly state that you either _accept_ or _reject_ the inclusion
> of this library into boost.
>
> I think Boost.Async should be _CONDITIONALLY ACCEPTED_ into Boost, provided that
> the following is addressed:
>
> - It should be possible to use generators without exceptions
> (https://github.com/klemens-morgenstern/async/issues/76)

I assume you meant channels?

> - All build errors and bugs reported during testing should be fixed and
> proper regression tests added for each of them.
> - BOOST_ASYNC_USE_STD_PMR macros are either removed or regression tested.
> - enable_await_allocator, enable_await_executor and friends are either
> hidden or expanded in the docs.
>
> Disclaimer: both Klemens and I are associated with the C++ Alliance. I'm
> writing this review trying to be as impartial as possible.
>
> Thanks Klemens for submitting and Niall for managing the review.
>

Thank you.

> Regards,
> Ruben.
>