Re: [boost] [outcome] Review of Outcome

Subject: Re: [boost] [outcome] Review of Outcome
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-05-29 14:38:39

>> > I don't consider expected<T, E> a particularly needed part of the >
>> library; the focus should be on result<T> and outcome<T>, the two >
>> classes that represent the error handling philosophy on which the >
>> library is built.
>>
>> FYI about half my potential user base want to set type E to their own
>> type, and see setting E to error_code as highly retrograde because it
>> loses type safety enforcement of disparate error code domains.
> 
> I don't object to expected<> being included; I just don't think that it
> has to have the spotlight. You're designing result<> and outcome<>, not
> expected<>, where you're tracking Vicente's papers. result/outcome
> represent a different paradigm.

Thing is, end users have clearly indicated that the majority isn't
interested in my or your or Boost flavours of Expected. They want "the"
Expected going into standards now.

Much of that is a lack of awareness of how standards operate, they think
that everything is like the Ranges TS in that you can be using Ranges
right now if you go git submodule
https://github.com/ericniebler/range-v3. They are looking for an
equivalent for Expected in a git submodule.

That's why the docs start from their perspective, and try to disabuse
them of that starting point because it is possibly short sighted for
their use case. I try to guide them from where a fair majority of them
are starting from, into converting their custom error code enum into a
proper error_category and using error_code instead.

That doesn't suit all custom error code type use cases e.g. Emil's, but
I completely agree with you that most of the time if they're not using
error_code, they should be.

>> I would recommend against standardising result<T> and outcome<T> in
>> the strongest terms. Expected is the right design for the STL, not
>> Outcome.
> 
> I don't agree here. One or both of result<T>/outcome<T> are a perfect
> fit for <system_error>. They expand the potential utility of
> std::error_code enormously and allow previously dual interfaces (f.ex.
> filesystem) to be expressed much more cleanly.
> 
> Nothing against expected<>, I hope that we get it right as well. Horses
> for courses.

I'd support an addition to <system_error> like you suggest *after*
Expected is into the standard and has had a bit of practice in the wild.

We are in agreement that narrow observers are probably daft in an object
mostly used for returning surprise. Vicente in the other thread appears
to be unwilling to accept my request that good API design should always
follow the principle of "less safety requires more programmer typing"
which in my mind means operator*() needs to be wide, as does .value()
and .error(). Let .unsafe_value() etc be the narrow editions. So I don't
think I can reconcile Outcome with Expected now.

But the reason for my caution regarding standardising std::result<T> is
that maybe expected<T, E = std::error_code> with its narrow observers
except for .value() will actually turn out to be fine in practice. You
might then say "but if it is not, then the Filesystem TS and Networking
TS with expected<T> return overloads will now have become unsafe" which
is a valid worry. But then I'd not retrofit those with Expected yet. Too
soon.

>> > I would also support chaining result<>s by storing the index of the
>> > parent ring buffer entry in the current ring buffer entry. This is a
>> > straightforward and very useful extension.
>>
>> It is an interesting idea, but as I said at the time not as useful as
>> you might think given the ephemeral nature of the storage.
> 
> It's not as useless as you might think.
> 
> Chaining results occurs during stack "unwinding" due to an error in a
> low level API. It returns a error result<>, the upper layer also returns
> an error, and so on upwards the stack, with some layers however deciding
> to replace the result<> with their own instead (as you do in your example.)
> 
> At the uppermost level, you get an error result<> and it'd be extremely
> valuable if when logging the error you could follow the chain and log
> all intermediate results.
> 
> For that, a ring buffer with 2048 entries (or however many you had) is
> plenty enough; there won't be 2048 failed operations in-between (well,
> there might be because threads, but you take what they give you; it
> can't get any worse than not having the information at all.)

I think our disagreement solely stems from the ring buffer being just 16
entries long. If it were 2048 entries, then yes, I'd agree with you.

>> I am still surprised that nobody has yet objected strongly to storage
>> which can vanish randomly during usage.
> 
> The ring buffer hasn't gotten much review love. I like the idea.
> Although, as I mentioned, I'd have separated it slightly more from the
> rest so that it can be excised or reworked if necessary without touching
> the main API of result/outcome.

As you may have noticed, I was misusing and abusing the lightweight
logger in boost-lite to shoehorn in a quick and dirty implementation. I
had been assuming that error_code_extended would be highly
controversial, and I didn't want to waste time on a better
implementation as I assumed I'd have to rip it out.

Now I know it is not controversial, and it would drop the final hard
dependency on boost-lite, it's definitely up for a proper local
implementation. As was obvious in the list of files given to Bjorn, it's
the only hard dependency for the header inclusion use case. The C++
exceptions disabled unit testing is the only other hard dependency
remaining for the entire library.

> Even so, dual licensing is still an unnecessary obstacle you're erecting
> before your library. Just use the BSL and be done with it. I don't
> understand you love of setting precedents. This, too, is steering
> committee material, as everything else in Boost is BSL.

Ah, but how boring would this review have been without a bit of thought
provoking implementation decisions? After all, this is me.

Firstly, thanks for the explanation above, it helped clarify things. I
appreciate that your answer to my final question is going to be "just
make it a normal Boost library", but let's say if you didn't answer
that, I'd be interested on whether you'd prefer one of these two options:

1. Cronjob generated boostorg/outcome repo from ned14/outcome repo
  - Licence only BSL
  - No git submodules
  - No cmake, just bjam
  - No file clutter in root of repo as at present, or files unrelated to
absolute minimum necessary for Boost

2. Same physical repo as standalone Outcome
  - Dual licensed
  - git submodules, but don't need to be checked out
  - cmake + bjam
  - Inevitable file clutter as so much tooling insists on files in
specific locations

Niall

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