
Hi Everyone, This is not my final review (yet), but I wanted to raise some issues with Regular ops: comparisons and swap. 1. Comparisons. Equality comparison for `result` makes a strange choice for `T`s without comparisons: two such `result<T>`s just compare unequal. This is somewhat contradictory to semantics of Regular types. For value-semantic types we expect that a copy of the original object `x` compares equal to `x`: ``` int i = 1; int j = i; assert (i == j); ``` For some types, copy-construction makes a copy with the same value, but there is no way to define operator==. In this case, in order not to confuse anyone operator== is simply not provided: ``` using transformation = std::function<int(int)>; transformation f = [](int i) { return i; }; transformation g = f; // assert (f == g); <-- does not compile ``` And now, when we wrap it into `result`, we get: ``` // continuing previous example: out::result<transformation> of = f, og = g; f == g; // <-- compiles fine, returns false. ``` Now you are just telling lies. Two objects have the same value, yet they compare unequal! Here is a live example: https://wandbox.org/permlink/I hqgXDfHW6WqAXHW My recommendation would be not to provide comparison operators if either `T` or `EC` does not provide them. 2. Swap For non-trivially-copyable types, it looks like (similarly to std::variant) `result` cannot provide the strong exception safety guarantee. You may need to both move-construct `T` and `EC` and both can fail. ``` void swap(result &o) noexcept(detail::is_nothrow_swappable<value_type>::value // &&detail::is_nothrow_ swappable<error_type>::value) { using std::swap; #ifndef BOOST_NO_EXCEPTIONS this->_state.swap(o._state); try { swap(this->_error, o._error); } catch(...) { swap(this->_state, o._state); throw; } #else swap(this->_state, o._state); swap(this->_error, o._error); #endif } ``` The trick with catch-reswap-rethrow assumes that `T` is more likely to throw while swapping/moving than `EC`, but it might be quite the opposite. Also, it is possible that while reswapping, another exception will be thrown. In general, you cannot guarantee the roll-back, so maybe it would be cleaner for everyone if you just declared that upon throw from swap, one cannot rely on the state of `result`: it should be reset or destroyed. Another problem is that in the noexcept() clause you are mentioning is_nothrow_swappable<value_type> and is_nothrow_swappable<error_type>, but in the implementation you might be using move constructors rather than swaps. This means that for types that do not throw in swap but throw in move constructor (like std::list), a throw from move constructor will cause a call to `std::terminate`. See live example here: https://wandbox.org/permlink/8jIyb0fx7U72cZSt The noexcept clause should be extended to include move-constructors. This is the case for `std::optional`. Regards, &rzej;