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-15 15:48:55

>> Similarly I intentionally sit on the fence with all the rest of
>> Outcome: it is up to the end user to choose the idiomatic style they
>> prefer. That's why the public API looks "heavy", but really it's just
>> many syntax ways of doing exactly the same thing.
> 
> The problem with this approach, apart from not sitting well with me
> personally, 

I do want to emphasise that I agree with you. Outcome is the only C++
library I've ever written which was multi-paradigm. Normally I decide on
one "right" way to do things, and enforce that on end users. You may
remember that from the AFIO v1 review where everyone didn't like my
choice of single vision, hence universal rejection bar one.

But in the end, I am overwhelmed by evidence that a single "right" way
is the wrong design here in this one very unusual situation. In my own
libraries which use Outcome I am using different conventions and use
patterns. I tried to summarise those with catchy names for the design
patterns in the Outcome tutorial, but to quote from
https://ned14.github.io/boost.outcome/md_doc_md_07-faq.html#examples_of_use:

"The main reason I designed and wrote Outcome was to implement a
universal error handling framework which could express in the minimum
possible overhead, and without losing information, the many C++ error
handling design patterns possible, and even more importantly that
individual libraries could use the design pattern which best suited them
whilst seamlessly interoperating with other libraries using different
error handling designs. To go into a bit more detail:

* Proposed Boost.AFIO v2 is a very low level very thin file i/o and
filesystem library which sits just above the raw kernel syscalls.
Throwing exceptions in such a library is overkill, so AFIO v2 uses the
"sea of noexcept" design pattern both in its public API and in its
internal implementation (i.e. it doesn't use C++ exceptions at all).

* Proposed Boost.KernelTest is a kernel based testing infrastructure
which uses Outcomes as the storage for each kernel permutation run in
its permutation tables of preconditions, postconditions, parameters and
outcomes. KernelTest itself is written using the "exceptions are
exceptional" design pattern where expected errors are returned via
outcomes but unexpected errors which abort the test use thrown
exceptions which are collected into outcome<T>'s. AFIO v2's test suite
is written using KernelTest.

* Planned Boost.BLOBStore will be a versioned, ACID transactional key to
BLOB store written using AFIO v2 which will use both the "sea of
noexcept" and the "exceptions are exceptional" design patterns together.
This allows user supplied callbacks to throw exceptions which aborts the
current transaction and for those exceptions to be propagated, if
desired, out of BLOBStore whilst the internal implementation of
BLOBStore and indeed its public API is all noexcept and never throws
exceptions (writing correct filesystem code is hard enough without
dealing with unexpected control flow reversal). BLOBStore will also use
KernelTest for its test suite."

I know you've read the history page Peter, so you know Outcome has
already had its API trimmed by half. These past 18 months I've been
removing stuff, paring down to the minimum. I don't claim I'm done
removing stuff yet, I agree there is a little more to go, but it's
getting harder to decide on what isn't really necessary any more.

> is that once the library enters wide use, you can no longer
> take any of these alternative APIs away without breaking code.

I absolutely agree with this assessment. Which is why Outcome's ABI is
versioned, and can be iterated with breaking changes if needs be.

Right now the ABI is permuted per git commit with the commit SHA as
Outcome is unstable. Once it's been firmed up through people using it
more, I'll declare a stable ABI which will be written in stone and
tested with abi-compliance-checker per commit. But thanks to the ABI
versioning, I can also make breaking changes without breaking code if it
turns out I made a terrible mistake in semantics.

> It's
> better, in my opinion, to provide a minimal interface first, then add
> bells and whistles on an as-needed basis. I look at result<>'s reference
> documentation and all I can think of is that 2/3 of it isn't needed and
> 2/3 of that even consists of different spellings of the same thing.

Are you referring to .get() and .value()?

I use a convention of .value() in my own code to indicate when I am
retrieving the value, and .get() to indicate I am throwing away the
fetched value but I do want any default actions to occur e.g. if not
valued, throw an exception. So .get() means "fetch and throw any error
state if present".

If people like this convention, I can make it formal by tagging
.value()'s return with [[nodiscard]] and have .get() return void. If
people dislike this convention, .get() can be removed.

I really wasn't sure what people might prefer. [[nodiscard]] is so new
it's hard to decide on if we are using it overkill or not. I look
forward to any feedback.

> I'd even remove value_or, there's nothing wrong with r? r.value(): def.

Both optional and expected have .value_or().

I've also found the ternary operator a poor substitute for .value_or()
in practice because both sides need to be the same type else the
compiler complains. .value_or() coerces the type properly. Less surprise
and less typing.

>> During my ACCU talk, about half the audience really want implicit
>> conversion for T and E in Expected so you can skip typing make_xxx()
>> all the time. The other half want always explicit instantiation so
>> that it is impossible to create an Expected *without* using make_xxx()
>> (BTW, Toronto will be discussing enforcing this for std::expected i.e.
>> to remove all implicit conversion)
> 
> A legitimate fork in the road. What I'd do is enable conversion when
> unambiguous, otherwise require make_expected<T> and make_unpexpected<E>.

I completely agree for the Expected proposal. But Vicente doesn't like
that idea.

Regarding the community disagreement being a legitimate fork in the
road, I used to think that. But as I hopefully demonstrated above, as I
design more libraries using Outcome I find myself using multiple
paradigms, and I think I am right to have done that for each library in
question. I claim therefore that this is not a fork in the road, but
rather parallel roads all starting from the same place and ending in the
same place.

One therefore needs an error handling system which easily lets you
"cross lanes" between those parallel roads simply and without losing
original information. You thus get Outcome.

>> Another defect in Expected in my opinion is having value return
>> semantics for .value_or(). You'll note Outcome's Expected has
>> reference return semantics for .value_or() which is a deviation.
> 
> I don't care for the pervasive &/&&/const&/const&& duplication
> (fourplication) very much myself, would return by value everywhere, but
> that's a matter of taste. Follows from the philosophy to provide the
> simple thing first, then complicate if need arises.

Alas Expected and Outcome permit usage with types with no default
constructor, no copy nor move. If this were not the case, I'd agree with
returning by value everywhere. But if you permit types limited like
that, returning by value anywhere in your API ought to be ruled out as
causing needless surprise and consternation to end users when an API
suddenly stops working just because the type used has changed.

BTW, is everyone aware that expected<void, void> is legal? Outcome
actually uses it internally. It is actually useful, believe it or not.

Niall

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