Hi, Cool to have a library like this.
I have a few nit-picks. I'd put the whole library in a namespace as it uses some quite common names like Result, Error and Future. future.h:15 The Result type could be made more API-compatible with the expected std::expected. Things like explicit operator bool() const noexcept; s/ok/ has_value/, and others. See [1] Also, you should probably add a static_assert(not std::is_same_v<T, E>) because of holds_alternative and std::get. Alternatively, you can use index-based checks and get. The value and error member functions should return const& to T and Error respectively instead of forcing a copy. (if you want a bit more than this, you can add && overloads that return T&& and Error&&). future.h:31 Replace is_same...::value with is_same_v for readability future.h:119 Is it needed to have a std::function there (having something wrapped into std::function and then that std::function wrapped in a lambda) - why not make the then member function a function template? (in other places as well) Also, naming both the function and the argument `then` is a bit icky. coroutine_integration.h:12 An overly common namespace name/alias `ns` exported by this header. coroutine_integration.h:37 t is default-constructed and then assigned a new default-constructed instance. Any reason for this? (same issue in other constructors in this file) What's more, most constructors in this file do not seem to serve a purpose. Cheers, Ivan [1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/ p0323r8.html#expected.synop -- dr Ivan Čukić i...@cukic.co, https://cukic.co/ gpg key fingerprint: 8FE4 D32F 7061 EA9C 8232 07AE 01C6 CE2B FF04 1C12
