[boost] [outcome] On the design and documentation

Subject: [boost] [outcome] On the design and documentation
From: Thomas Heller (thom.heller_at_[hidden])
Date: 2017-05-24 19:44:29

Hi Niall (and also probably Vicente)

I was following the discussion about expected/outcome/result very closely and 
I can absolutely see the usefulness of such a library (I explicitly discarded 
option here).

I am not sure if this post should count as a review, mainly because I disagree 
with the fundamental design decisions (that includes expected as defined in 
D0323R2) and therefor would cast my vote as in "not ready yet", I hope I can 
convey my points below.

First of all, I don't agree with the strong (conceptual) relationship between 
optional (be it boost:: or std::experimental) in such a way that expected is a 
generalization of it. From what I understand, the purpose of expected/outcome/
result is to be used as a mechanism to return the result of a function. As 
such it should also expose the semantics of it. Fortunately, we already have a 
(asynchronous) return object standardized (std::future). And this is my basic 
disagreement here: Why not model expected in the lines of future? With the 
main points being: 
 - expected being movable only
 - expected<T,E>::value() should always return by value and invalidate this.
 - (I would really prefer the color .get though ;))
So the question to Vicente/Niall is: what is the motivation to make it 
"optional-ish"? Do we have use cases which make this design undesirable?

By having these constraints, expected of course needs to have an uninitialized 
state. As such we'd have the three observers: valid(): true when has_value() 
|| has_error(), false otherwise (for example default constructed, 
invalidated), has_value() and has_error().

Second, I think Niall raised a valid about outcome being a framework for 
interoperability (completely orthogonal to the first point). However, I totally 
miss this from the proposed library, most pressing are non intrusive 
mechanisms. For that purpose I postulate, that a mechanism to transform 
between different unexpected results, that is: various error codes etc.
However, for that to work, one would of course need a properly defined concept, 
for example, as Vicente suggested "EitherValue", and a mechanism to coerce one 
error type into another, maybe through ADL, or traits specialization or 
whatever.
With that in place, one could simply define the different EitherValue types, 
there is no need that everything needs to be in the form of "basic_XXX". For 
the library under review, this would be perfectly sufficient:
template <typename T, typename E>
class expected;
template <typename T>
using result = expected<T, extended_error_info>;
template <typename T>
using outcome = expected<T, variant<extended_error_info, exception_ptr>>;

That is, given that we have either a value or unexpected, we can convert 
expected<T, E1> to expected<U, E2> if T is convertible to U and E1 "coercable" 
(with whichever mechanism) to E2.

If we then have a generic mechanism to get from a (possibly user defined "E") 
to an exception, I completely miss the point of the outcome template.

Furthermore, I believe that .value/.error should really have a narrow 
contract, that is that it is UB to call those functions if the respective 
types are not held (easy to catch this in a debug build...). Why? Probably 
just a micro optimization, but consider the canonical usage:
auto r = some_function_returning_expected(...);
if (r.has_value())
    // Do something with the value
else
    // An error occurred, PANIC
In both branches, we know exactly what's in there ... so why check again when 
getting the state out? I don't get the "reinterpret_cast" argument. I am one 
of the persons that believe that UB is a necessary evil for some 
optimizations...

All optimizations can then easily be put as implementation details and the 
generic expected<T, E> will probably suffice for most use cases, for everything 
else, we can implement special types which conform to our concepts and 
implement the error conversion mechanisms. This will most likely also work 
with different APIs/ABIs.

Last but not least, I wanted to give some feedback on the proposed 
Boost.Outcome documentation. At the very least, I find it highly confusing.
When I open its "landing page", I get no idea about the following:
- What is the purpose of the library?
- Why/when should I use it?
- What should I look into next?

>From the front page, it looks like a implementation of expected, but only 
talks about the outcome template, it is very hard to find the documentation for 
the other three types result, outcome and option. Yet, the documentation 
explicitly warns about it being unstable and suggests to use its refinements.
Useful information is spread across the whole documentation, there is no easy 
way to find the requirements on T or E. The reference section is anything but 
complete and doesn't convey any useful information.

I did not really look at the actual implementation of the proposed library. I 
cloned the git repository and had a quick glance, but to be honest, I got lost 
very early, I have no idea where to start looking first.

Cheers,
Thomas