Re: [boost] [review] Review of Outcome (starts Fri-19-May)

Subject: Re: [boost] [review] Review of Outcome (starts Fri-19-May)
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-05-21 10:47:49

>> And finally, I do intend to keep Outcome working completely
>> independent of Boost. The vast majority of my user base explicitly do
>> not want a Boost dependency.
> 
> The vast majority of your user base may change if your library is
> accepted. It almost certainly will.

I would still expect a vast majority to not want a Boost dependency.
People want C++ 14 libraries to use the C++ 14 STL, not Boost, though
*optional* non-default usage of Boost stuff appears to be popular.

> Not wanting to depend on Boost is understandable. Our common
> infrastructure however is not just a whim of ours, it's a service to
> Boost users. Going through BOOST_ASSERT and BOOST_THROW_EXCEPTION
> instead of rolling your own is more convenient for people who have
> already configured BOOST_ASSERT and BOOST_THROW_EXCEPTION to do what
> they want them to do and would rather not repeat that exercise for every
> new library, however wonderful.

Outcome only ever throws an exception via a macro. It can be customised.
If reviewers wish it, we can have the default use BOOST_THROW_EXCEPTION.

> Similarly, once we get the kinks out of Boost.Stacktrace, it might be
> better to use that instead of #including <windows.h>.

Including Outcome does not include <windows.h>, except on winclang
because winclang won't compile my win32 namespace anti-collision hack
which MSVC will.

The stack trace optionally collected by error_code_extended is
completely standard and can be fed to Boost.Stacktrace, or to any other
stack trace library. The default printer provided by Outcome is
rudimentary, but sufficient. I personally feed the addresses printed to
addr2line by hand when I need to, it works. I would be very open to
using Boost.Stacktrace instead if detected with #if _has_include

> All that's not strictly required for acceptance, in principle; neither
> is using Boost.Config instead of rolling your own. And yet.

Boost-lite macros can be swapped for Boost.Config ones if reviewers wish it.

I should mention that boost-lite derives its macros from the C++ 17
feature macros, which all recent editions of major compilers support
even in C++ 14 mode. In other words, it is not compiler version based,
except on old compilers, but it does rely on compiler vendors telling
the truth (they generally do).

> Regarding the ring buffer, I agree that it adds value. But it also adds
> complexity. There's an argument to be made for result/outcome classes
> that simply store std::error_code without all the bells and whistles,
> and provide an optional hook to which the user can attach the ring
> buffer if needed.

The tutorial covers the need in real world code to attach some sort of
payload to error codes. Outcome provides error_code_extended with a
reasonable set of the most common likely payloads.

If reviewers would prefer a vanilla error_code to be the default, that's
very easy to fix. I would add that if you do not use any of the payload,
error_code_extended is identical to an error_code, no overhead.

> Finally, you don't even need to macro-bind to std::error_code. Inspired
> by Outcome, I wrote a pull request for Boost.System which makes the
> Boost error_category, error_code and error_condition convertible to the
> standard ones, so you can just store std::error_code and be done with it.
> 
> https://github.com/boostorg/system/pull/16

I'll leave it to reviewers to decide on whether defaulting to the C++ 14
STL std::error_code or to boost::error_code is the most appropriate.
It's very easy to change. And as I've mentioned before, if reviewers
really want a completely boostified edition of Outcome, that's no
problem, I can make that work.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/