The coro::task should not use std::optional
a858438680 opened this issue · 2 comments
Now the coro::task
uses the std::optional
to store the result, which is incorrect.
The optional is designed to store alterable values, so it requires that the type it stores is copy/move constructible and assignable. However, a normal function can definitely return a value with all these functions deleted.
Let's think about what do we need to return a value from the coro::task
. First, the task needs to store the return value inside the promise, and then construct the return value in the caller's frame. So, the requirement of std::optional
is too strong and does not meets what we need here. I suggest that we use a custom nullable type to replace this std::optional
. A reference implementation is like this
template <typename T>
struct nullable {
public:
using stored_type = std::remove_cv_t<T>;
nullable() noexcept: m_engaged(false) {}
nullable(const nullable&) = delete;
nullable(nullable&&) = delete;
nullable& operator=(const nullable&) = delete;
nullable& operator=(nullable&&) = delete;
~nullable() noexcept(std::is_nothrow_destructible_v<stored_type>) {
if (has_value()) {
mut_value().~stored_type();
}
}
template <typename... Args>
void emplace(Args&&... args) noexcept(std::is_nothrow_constructible_v<stored_type, Args...>) {
new (m_payload) stored_type(static_cast<Args&&>(args)...);
m_engaged = true;
}
bool has_value() const noexcept {
return m_engaged;
}
const stored_type& value() const& noexcept {
return *std::launder(reinterpret_cast<const stored_type*>(m_payload));
}
stored_type&& value() && noexcept {
return static_cast<stored_type&&>(*std::launder(reinterpret_cast<stored_type*>(m_payload)));
}
private:
alignas(stored_type) unsigned char m_payload[sizeof(stored_type)];
bool m_engaged = false;
stored_type& mut_value() & noexcept {
return *std::launder(reinterpret_cast<stored_type*>(m_payload));
}
};
And by the way, the empty or has_value flag can be combined with whether there is an exception flag. So that the std::exception_ptr
can share the storage with the value. I suggest that we use this solution to further reduce the size of task. For example, when return type is uint64_t, std::exception_ptr
+ uint64_t
+ bool
is acctually 24 bytes (because of padding), but if the std::exception_ptr
and uint64_t
shares storage, 8 + enum(unsigned char) is 16 bytes.
If you think these changes are good, I will soon create a new pull request which make these changes.
I think we can close this one out? Is there anything else you want to include?
I think we can close this one out? Is there anything else you want to include?
Yes, I think we can close it.