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