[boost] [outcome] Review of Outcome

Subject: [boost] [outcome] Review of Outcome
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2017-05-28 22:43:34

I. Design

There is a need in C++ for dealing with failures without using exceptions.
This is not so much driven by actual inefficiencies (as opposed to
perceived inefficiencies) of exception handling, but by the fact that not
all C++ code in existence is exception-safe, and the fact that some C++
programmers work in environments where exception handling is disabled, as
ill-advised such a decision may be.

In my opinion the library should drop option<T> and result<T> altogether
and should be reduced to outcome<T> only, which can be empty, or hold a T,
or hold an exception (but not error code, see below.) This is critical for
a library that aims to provide a _common_ general mechanism for dealing
with failures in interfaces; providing a rich set of alternatives works
against that goal.

(It seems desirable to get rid of the empty state too but the discussions
during the review period show that this might cause more problems than it
solves.)

Secondly, it is not a good idea to use error codes or any other value as
means to dispatch on different kinds of failures because semantically such
dispatch should be static: there is some _code_ which detects the error,
and there is some other _code_ that needs to bind it and handle it. This is
one reason why in C++ catch dispatches by the static type of the exception
objects, not by some exception.what() value.

Further, using static types to communicate different kinds of failures
allows users to recognize and handle an entire class of errors by means of
implicit type conversions, by organizing error types in a hierarchy. For
example:

struct io_error: virtual std::runtime_error { };
struct read_error: virtual io_error { };
struct write_error: virtual io_error { };
struct file_error: virtual io_error { };
struct file_read_error: virtual file_error, virtual read_error { };
struct parse_error: virtual std::runtime_error { };
struct syntax_error: virtual parse_error { };

With this hierarchy, we can use read_error to handle any kind of read
errors (not just file-related), file_error to handle any kind of file
failures (read or write), etc.

That said, outcome<T> should also be able to transport values, however
their purpose should not be to tell _what_ went wrong, but to provide
additional information.

It must be stressed that such additional information should be decoupled
from the classification of the error. For example:

outcome<parsed_data> parse_file( char const * file_name );

The above function reads a file and returns some parsed_data -or- stores an
exception object in the outcome. What are the possible failures? It may be
a file_read_error, but also types unrelated to I/O like std::bad_alloc or
syntax_error. Regardless of the type of the failure, the outcome object
should be able to transport the file name because it is relevant no matter
_what_ went wrong.

Incidentally, the need to transport contextual information independently of
the error classification exists when throwing exceptions as well, so if
outcome<T> can hold exception objects (and allow users to dispatch on their
static type, like catch does), the exception objects themselves can take
care of holding any additional information, for example by using Boost
Exception.

II. Implementation

In general the library should not shy away from integrating with common
Boost facilities in an effort to appeal to people who hate Boost. A Boost
library should primarily target Boost users and address their concerns.

This includes the testing framework as well as boost/config.hpp where
applicable (it is okay to not always include boost/config.hpp, but it
should be included to get things like BOOST_NO_EXCEPTIONS and other
configuration macros.)

The library should use BOOST_ASSERT, though I don't see a problem with not
#including boost/assert.hpp if BOOST_ASSERT is already defined (by the
user).

The library should use BOOST_THROW_EXCEPTION to throw. Like in the case of
BOOST_ASSERT, it's fine to not #include boost/throw_exception.hpp if
BOOST_THROW_EXCEPTION is already defined.

That said, avoiding coupling with individual Boost submodules is desirable
in general. Also there is no problem with providing facilities to avoid all
Boost coupling in order to support non-Boost users.

III. Documentation

The documentation makes broad claims about the inefficiency of exception
handling which seem to be motivated by a desire to appeal to programmers
who hold such beliefs. There is no need to make such generally untrue
claims in order to justify the need for Outcome.

Other than that the documentation does the job.

IV. Verdict

Should Outcome be accepted into Boost? NO

This obviously applies only to the current state of the library.