Subject: [boost] [review] [Outcome] Deniz' review
From: Deniz Bahadir (dbahadir_at_[hidden])
Date: 2017-05-24 11:33:49
this is my review of Niall's Outcome library.
It is my fist review, so better keep that in mind when considering my
answers for Outcome's acceptance into Boost.
> The formal review of Niall Douglas' Outcome library starts today
> (Fri-19-May) and continues until Sun-28-May.
> Your participation is encouraged, as the proposed library is uncoupled and
> focused, and reviewers don't need to be domain experts to appreciate the
> potential usefulness of the library and to propose improvements. Everyone
> needs (and has suffered) error handling, and can compose an opinion on that
> Outcome is a header-only C++14 library providing expressive and type-safe
> ultra-lightweight error handling, suitable for low-latency code bases.
I think it is a good decision to target a more modern C++ standard,
especially if it eases the implementation. (I am not such a big fan of
re-implementing what is already in a particular standard, just to
support old compilers. At least not for a new library.)
> Key features:
> *- Makes using std::error_code from C++11's <system_error> more convenient
> and safe
> *- Provides high-quality implementation of proposed std::expected<T,E> (on
> C++20 standards track)
> *- Good focus on low-latency (with tests and benchmarks)
> *- Error-handling algorithmic composition with-or-without C++ exceptions
> *- No dependencies (not even on Boost)
The last point, I think, is not really bad.
Even if Outcome will be accepted into Boost and therefore becomes a part
of Boost I think it is good idea to only depend on other Boost modules
if it is absolutely necessary. (For example, I would not consider anyone
re-inventing Boost.Hana. But if a simple Tribool class is needed, I am
all for providing a simple one, especially, if it is just an
And as the library targets C++14 I am all for using smart-pointers from
the standard instead of having a dependency on Boost.SmartPtr or
> This review is timely, as C++17 brings us std::optional<T>. The upcoming
> std::expected<T,E> (an implementation provided in Outcome) is a
> generalization of std::optional<T> that provides a <success|fail> value,
> where the unhappy result is a 'std::error_code' or an instance of
> The library further provides 'outcome<T,error-code,exception-ptr>' for
> handling <success|error|exception> to safely wrap throwing APIs.
> Here are some
> questions you might want to answer in your review:
> - What is your evaluation of the design?
I like it. The idea of combining different error-/return-code and
exception handling seems great to me.
Ensuring, that it can be compiled into very efficient machine-code is a
very nice plus.
However, I would like Niall to recheck if several of his
code-optimizations that make the code hard to understand or maintain are
still really needed for the set of supported compilers. As was mentioned
in previous email-discussions this might not always be the case.
(Maybe, it is even possible instead to provide some hints to
compiler-vendors where to try to improve their optimizers?)
> - What is your evaluation of the implementation?
It fascinates me to see so many macros in headers which auto-generate
the names of header-files to be included. I personally like such
automatisms where one only has to modify the values of some few macros
in a single file to get a totally different behavior or implementation.
(I myself happen to implement such automatisms quite often.)
However, that always carries the risk of confusing the user/reviewer (or
the future maintainer) who tries to understand what is going on. In
general, this might be solvable with even more documentation/comments,
but is still more work to grasp what's going on. However, a general user
might not care at all as long as everything works fine out of the box.
(So let's hope it always does.)
Other than that, the implementation looks solid. Although, I must
confess that I did not look into it in-depth.
(It at least seems to be better documented with comments than many other
Boost-libraries that I have looked into and tried to understand what is
> - What is your evaluation of the documentation?
The "tutorial" is quite long but also quite good. I think I understood
everything, because it explains most things in great detail. That's, why
I would not consider it a "tutorial" but an in-depth documentation.
I would expect a tutorial being more of a short introduction which
explains the basic usage in not more than 3 to 4 screen-pages.
The "landing-page" is more what I would have thought a tutorial would
look like; a little longer maybe with smaller examples and some more
text between them.
But then again, that could just be me having a wrong impression what a
tutorial in general should look like.
At least, the entire documentation (and also the link to the ACCU-talk)
helped me to understand what Outcome is for.
> - What is your evaluation of the potential usefulness of the library?
I think, it is very useful.
I myself would probably just need the main functionality "expected"
provides (value or error) but having some extensions which can handle
exceptions, too, is a good thing.
With "outcome" and "result" being possibly empty I am not so sure. It
probably is really better to report the empty-case through the
error-code. (But I think, Niall is already considering this.)
The benefit of having "option" is not totally clear to me. It looks
similar to "std::optional". Is its implementation just more efficient
than the one found in different standard-libraries? (Then again,
std::optional comes with C++17 and Outcome targets C++14.) How about
Boost.Optional? Would it make sense to have a dependency on it instead
(and possibly improve its implementation with some tricks of Outcome's
If "outcome" and "result" report the empty-case through the error-code
then that would make "option" obsolete, because one could use "result"
instead with an error_code that will be set to empty_value (or how it
would be called). That would also remove a possible dependency on
> - Did you try to use the library? With what compiler? Did you have any
No, sorry, I did not.
> - How much effort did you put into your evaluation? A glance? A quick
> reading? In-depth study?
Several hours reading the docs, watching the ACCU-talk (and reading the
mail-discussions on this mailing-list). A quick look into the code.
> - Are you knowledgeable about the problem domain?
Just as much as a developer could be who has worked with code that uses
exceptions, error-codes, return-codes and has lazy-co-developers (me
included) that forget to check the return-codes or to catch exceptions.
> And most importantly:
> - Do you think the library should be accepted as a Boost library?
Yes, I would say so.
This would be an unconditional yes, even though I would suggest to add a
small "introductory-tutorial" to the documentation and would like to
hear what Niall (and others) think about my above remarks concerning
removal of "option" and re-checking of the specific compiler-optimizations.
> For more information about Boost Formal Review Process, see:
> Thank you very much for your time and efforts.
Thank you very much for putting so much work into Outcome, Niall, and
Charley for being its review-manager.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk