Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions include/seastar/core/coroutine.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@

#ifndef SEASTAR_MODULE
#include <coroutine>
#ifdef __cpp_lib_expected
#include <expected>
#endif
#endif

namespace seastar {
Expand Down Expand Up @@ -130,6 +133,59 @@ public:
};
};

#ifdef __cpp_lib_expected
template <typename Err>
class coroutine_traits_base<std::expected<void, Err>> {
public:
class promise_type final : public seastar::task {
seastar::promise<std::expected<void, Err>> _promise;
public:
promise_type() = default;
promise_type(promise_type&&) = delete;
promise_type(const promise_type&) = delete;

template<typename... U>
void return_value(U&&... value) {
_promise.set_value(std::forward<U>(value)...);
}

void return_value(std::expected<void, Err>&& value) {
_promise.set_value(std::forward<std::expected<void, Err>>(value));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it (and we want to move from boost::outcome to std::expected too).

But there's too much copying here. Perhaps add this to the main template with a constraint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avikivity are you infavor of #2977? If so, I'll close this patch as it also handles the case.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to do it deliberately in its own pull request rather than sneak it in as part of another change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the fundamental issue is that the compiler can't infer the return type because of the template in return_value so if we want to move to return_value not being a template then this case doesn't need to be specialized anymore.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok


void return_value(coroutine::exception ce) noexcept {
_promise.set_exception(std::move(ce.eptr));
}

void set_exception(std::exception_ptr&& eptr) noexcept {
_promise.set_exception(std::move(eptr));
}

void unhandled_exception() noexcept {
_promise.set_exception(std::current_exception());
}

seastar::future<std::expected<void, Err>> get_return_object() noexcept {
return _promise.get_future();
}

std::suspend_never initial_suspend() noexcept { return { }; }
std::suspend_never final_suspend() noexcept { return { }; }

virtual void run_and_dispose() noexcept override {
auto handle = std::coroutine_handle<promise_type>::from_promise(*this);
handle.resume();
}

task* waiting_task() noexcept override { return _promise.waiting_task(); }

scheduling_group set_scheduling_group(scheduling_group new_sg) noexcept {
return task::set_scheduling_group(new_sg);
}
};
};
#endif

template<bool CheckPreempt, typename T>
struct awaiter {
seastar::future<T> _future;
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/coroutines_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1018,3 +1018,17 @@ SEASTAR_TEST_CASE(test_lambda_coroutine_in_continuation) {
}));
BOOST_REQUIRE_EQUAL(sin1, sin2);
}

#ifdef __cpp_lib_expected

future<std::expected<void, std::string>> void_return_expected() {
co_return {};
}

SEASTAR_TEST_CASE(test_std_expected_void_specialization) {
auto result = co_await void_return_expected();
BOOST_REQUIRE(result.has_value());
}

#endif