From 6ab08aa227509c21d3ebdab6aadd1ca748a9843a Mon Sep 17 00:00:00 2001 From: Amber Ehrlich Date: Thu, 11 Jul 2024 10:11:08 -0400 Subject: [PATCH] improv(coro): add concept `awaitable_type`, move `dpp::detail::promise::promise` to `dpp::basic_promise`, document some more --- include/dpp/coro/awaitable.h | 101 ++++++++++++++++++++++++----------- include/dpp/coro/coro.h | 22 ++++++-- 2 files changed, 90 insertions(+), 33 deletions(-) diff --git a/include/dpp/coro/awaitable.h b/include/dpp/coro/awaitable.h index 1081a6cdd2..9ae78353ee 100644 --- a/include/dpp/coro/awaitable.h +++ b/include/dpp/coro/awaitable.h @@ -37,12 +37,12 @@ struct awaitable_dummy { #include +// Do not include as coro.h includes or depending on clang version #include #include #include -#include +#include #include -#include namespace dpp { @@ -91,20 +91,33 @@ class promise_base; */ struct empty{}; +/** + * @brief Variant for the 3 conceptual values of a coroutine: + */ +template +using result_t = std::variant, empty, T>, std::exception_ptr>; + template void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result); } /* namespace detail::promise */ -template -requires (requires (Derived t) { detail::co_await_resolve(t); }) +template class basic_awaitable { protected: + /** + * @brief Implementation for sync_wait. This is code used by sync_wait, sync_wait_for, sync_wait_until. + * + * @tparam Timed Whether the wait function times out or not + * @param do_wait Function to do the actual wait on the cv + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. + */ template auto sync_wait_impl(auto&& do_wait) { using result_type = decltype(detail::co_await_resolve(std::declval()).await_resume()); using storage_type = std::conditional_t, detail::promise::empty, result_type>; - using variant_type = std::variant; + using variant_type = detail::promise::result_t; variant_type result; std::condition_variable cv; @@ -131,6 +144,8 @@ class basic_awaitable { * @brief Blocks this thread and waits for the awaitable to finish. * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ auto sync_wait() { return sync_wait_impl([](std::condition_variable &cv, auto&& result) { @@ -145,8 +160,8 @@ class basic_awaitable { * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. * @param duration Maximum duration to wait for - * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. - * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ template auto sync_wait_for(const std::chrono::duration& duration) { @@ -162,8 +177,8 @@ class basic_awaitable { * * @attention This will BLOCK THE THREAD. It is likely you want to use co_await instead. * @param time Maximum time point to wait for - * @retval If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. - * @retval If T is non-void, returns a std::optional for which an absense of value means timed out. + * @return If T is void, returns a boolean for which true means the awaitable completed, false means it timed out. + * @return If T is non-void, returns a std::optional for which an absence of value means timed out. */ template auto sync_wait_until(const std::chrono::time_point &time) { @@ -339,6 +354,9 @@ class awaitable : public basic_awaitable> { namespace detail::promise { +/** + * @brief Base class defining logic common to all promise types, aka the "write" end of an awaitable. + */ template class promise_base { protected: @@ -377,6 +395,12 @@ class promise_base { } } + /** + * @brief Unlinks this promise from its currently linked awaiter and returns it. + * + * At the time of writing this is only used in the case of a serious internal error in dpp::task. + * Avoid using this as this will crash if the promise is used after this. + */ std_coroutine::coroutine_handle<> release_awaiter() { return std::exchange(awaiter, nullptr); } @@ -393,6 +417,8 @@ class promise_base { /** * @brief Move construction is disabled. + * + * awaitable hold a pointer to this object so moving is not possible. */ promise_base(promise_base&& rhs) = delete; @@ -412,6 +438,7 @@ class promise_base { * * @tparam Notify Whether to resume any awaiter or not. * @throws dpp::logic_exception if the promise is not empty. + * @throws ? Any exception thrown by the coroutine if resumed will propagate */ template void set_exception(std::exception_ptr ptr) { @@ -427,9 +454,12 @@ class promise_base { /** * @brief Notify a currently awaiting coroutine that the result is ready. + * + * @note This may resume the coroutine on the current thread. + * @throws ? Any exception thrown by the coroutine if resumed will propagate */ void notify_awaiter() { - if (state.load(std::memory_order::acquire) & sf_awaited) { + if ((state.load(std::memory_order::acquire) & sf_awaited) != 0) { awaiter.resume(); } } @@ -449,6 +479,8 @@ class promise_base { } }; +} + /** * @brief Generic promise class, represents the owning potion of an asynchronous value. * @@ -459,10 +491,10 @@ class promise_base { * @see awaitable */ template -class promise : public promise_base { +class basic_promise : public detail::promise::promise_base { public: - using promise_base::promise_base; - using promise_base::operator=; + using detail::promise::promise_base::promise_base; + using detail::promise::promise_base::operator=; /** * @brief Construct the result in place by forwarding the arguments, and by default resume any awaiter. @@ -479,9 +511,9 @@ class promise : public promise_base { } catch (...) { this->value.template emplace<2>(std::current_exception()); } - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); + [[maybe_unused]] auto previous_value = this->state.fetch_or(detail::promise::sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { - if (previous_value & sf_awaited) { + if (previous_value & detail::promise::sf_awaited) { this->awaiter.resume(); } } @@ -510,11 +542,20 @@ class promise : public promise_base { } }; + +/** + * @brief Generic promise class, represents the owning potion of an asynchronous value. + * + * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. + * As such, the promise needs to be kept alive for the entire time a value can be retrieved. + * + * @see awaitable + */ template <> -class promise : public promise_base { +class basic_promise : public detail::promise::promise_base { public: - using promise_base::promise_base; - using promise_base::operator=; + using detail::promise::promise_base::promise_base; + using detail::promise::promise_base::operator=; /** * @brief Set the promise to completed, and resume any awaiter. @@ -525,27 +566,30 @@ class promise : public promise_base { void set_value() { throw_if_not_empty(); this->value.emplace<1>(); - [[maybe_unused]] auto previous_value = this->state.fetch_or(sf_ready, std::memory_order::acq_rel); + [[maybe_unused]] auto previous_value = this->state.fetch_or(detail::promise::sf_ready, std::memory_order::acq_rel); if constexpr (Notify) { - if (previous_value & sf_awaited) { + if (previous_value & detail::promise::sf_awaited) { this->awaiter.resume(); } } } }; -} - -template -using basic_promise = detail::promise::promise; - /** - * @brief Base class for a promise type. + * @brief Generic promise class, represents the owning potion of an asynchronous value. + * + * This class is roughly equivalent to std::promise, with the crucial distinction that the promise *IS* the shared state. + * As such, the promise needs to be kept alive for the entire time a value can be retrieved. + * + * The difference between basic_promise and this object is that this one is moveable as it wraps an underlying basic_promise in a std::unique_ptr. * - * Contains the base logic for @ref promise, but does not contain the set_value methods. + * @see awaitable */ template class moveable_promise { + /** + * @brief Shared state, wrapped in a unique_ptr to allow move without disturbing an awaitable's promise pointer. + */ std::unique_ptr> shared_state = std::make_unique>(); public: @@ -712,9 +756,6 @@ namespace dpp { namespace detail::promise { -template -using result_t = std::variant, empty, T>, std::exception_ptr>; - template void spawn_sync_wait_job(auto* awaitable, std::condition_variable &cv, auto&& result) { [](auto* awaitable_, std::condition_variable &cv_, auto&& result_) -> dpp::job { diff --git a/include/dpp/coro/coro.h b/include/dpp/coro/coro.h index 76f3256ed8..cf5ab2cb51 100644 --- a/include/dpp/coro/coro.h +++ b/include/dpp/coro/coro.h @@ -114,13 +114,14 @@ decltype(auto) co_await_resolve(T&& expr) noexcept { } #else + /** * @brief Concept to check if a type has a useable `operator co_await()` member * * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_co_await_member; +inline constexpr bool has_co_await_member; /** * @brief Concept to check if a type has a useable overload of the free function `operator co_await(expr)` @@ -128,7 +129,7 @@ bool has_co_await_member; * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_free_co_await; +inline constexpr bool has_free_co_await; /** * @brief Concept to check if a type has useable `await_ready()`, `await_suspend()` and `await_resume()` member functions. @@ -136,7 +137,15 @@ bool has_free_co_await; * @note This is actually a C++20 concept but Doxygen doesn't do well with them */ template -bool has_await_members; +inline constexpr bool has_await_members; + +/** + * @brief Concept to check if a type can be used with co_await + * + * @note This is actually a C++20 concept but Doxygen doesn't do well with them + */ +template +inline constexpr bool awaitable_type; /** * @brief Mimics the compiler's behavior of using co_await. That is, it returns whichever works first, in order : `expr.operator co_await();` > `operator co_await(expr)` > `expr` @@ -144,6 +153,7 @@ bool has_await_members; * This function is conditionally noexcept, if the returned expression also is. */ decltype(auto) co_await_resolve(auto&& expr) {} + #endif /** @@ -154,6 +164,12 @@ using awaitable_result = decltype(co_await_resolve(std::declval()).await_resu } // namespace detail +/** + * @brief Concept to check if a type can be used with co_await + */ +template +concept awaitable_type = requires (T expr) { detail::co_await_resolve(expr); }; + struct confirmation_callback_t; template