Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)

Subject: Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2018-01-30 21:07:05

This is my review of Outcome v2.

- What is your evaluation of the design?

The design appears to not have been finalized, the documentation states as
much:

"As patches and modifications are applied to this library, namespaces get
permuted in order not to break any backward compatibility. At some point
namespace outcome::v2 will be defined, and this will be the prefered
namespace. Until then OUTCOME_V2_NAMESPACE denotes the most recently
updated version, getting closer to outcome::v2."

It is wise to take the author seriously on his intentions to evolve, rather
than refine, the design further. During the first review, changes were
happening in response to feedback received on the mailing list. The current
design is even less concrete with the addition of policies. To me, this is
a sign of uncertainty in the design. A hallmark of excellence in library
design is that at some point, if a user says "it would be nice to be able
to do X with this library" the answer is "no". I think Outcome's design is
driven by a desire to always say "yes".

The policy-based design is especially problematic because of the expressed
desire to provide interoperability between diverse APIs each using
different error reporting. This is a bit counter-intuitive, because the
natural inclination is to provide maximum flexibility, but in this case
other considerations are more important. There is a reason why in C
libraries the default error reporting mechanism is to return int, even
though the language does permit programmers to return structs, which would
be more flexible.

Speaking of interoperability, it is especially tricky to report errors from
C-style callbacks. This would be nice to support because C++ exceptions are
off-limits in this case, yet it is sometimes desirable to communicate
user-specific information across the third-party C callback mechanism (this
is sometimes supported by a void * user data pointer, but that is not
always the case).

The kind of objects that can survive crossing API boundaries would be of
basic types, or have one or two members of basic types. Think
std::shared_ptr<T>, which _always_ consists of two pointers, rather than
SmartPtr<T,Policy1,Policy2> which consist of who knows what. Note that this
is not the same problem as "using result<T> from C code", which Outcome
allows. The important question is not how do I use result<T> from C, but
how do I transport result<T> across a third party context which knows
nothing about Outcome (as a side note, C++ exceptions also cause issues
when crossing API boundaries, even if we set exception safety requirements
aside.)

Further, I disagree with the motivation to avoid using exceptions to begin
with. The supplied decision matrix, I think, does not reflect reality. In
my experience, the decision matrix to use C++ exceptions vs. something else
is much simpler: "Do you hate exceptions?" No => use exceptions, Yes => use
something else, because exceptions suck.

Yes, exception handling has overhead. No, you can't afford this overhead in
every last corner of a complex program, but yes, you can afford it in
general. I have had many discussions on this and other "but but but
Overhead" topics, and I have never been given actual evidence that any
given program that does not use exceptions to report errors could not be
written using exceptions to report errors, without sacrificing performance.
And this is trivially true: if exception handling causes problems in some
subsystem, the solution is to hide that subsystem behind a C-style API and
compile only that subsystem with exception handling disabled.

So, the design of "an error handling library that does not use C++
exceptions" has to target the tricky bits where exceptions would be
annoying or can't be used, not the general case where they can.

Finally, I'll point out that a lot of the positive feedback comes from
people who think that it is a good idea to replicate the Rust error
reporting mechanism in C++. This seems to be an axiomatic belief, since
I've never seen anyone attempt to substantiate it. This is important, as it
is typical for programmers coming from other languages to be shocked by
various C++ language features, and this should not be confused with
problems in the C++ language specification.

- What is your evaluation of the implementation?

Lack of C++11 support could be problematic. The use of macros to
disambiguate namespace is cumbersome. Overall the library relies heavily on
macros, which is not a good thing for a C++ library.

It seems like Outcome wants to stay away from Boost. By that I mean that
great care has been taken to decouple it from Boost, and I sense that this
desire comes from the target audience: the so-called "low latency" crowd
wouldn't touch Boost with a 9 foot pole. While it is generally a bad idea
to speculate about such things, it seems to me the motivation for
submitting Outcome for a Boost review is not to benefit the Boost
community; for us the coupling with Boost is not a problem.

- What is your evaluation of the documentation?

The documentation seems complete.

- Are you knowledgeable about the problem domain?

Yes.

- Do you think the library should be accepted as a Boost library?

The library should be rejected.

Emil