On Fri, Aug 18, 2023 at 5:59 AM Ruben Perez via Boost <boost@lists.boost.org> 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.