Re: [boost] Outcome/expected/etc/etc/etc

Subject: Re: [boost] Outcome/expected/etc/etc/etc
From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2017-06-06 09:22:14

2017-06-06 11:04 GMT+02:00 Niall Douglas via Boost <boost_at_[hidden]>:

> >> I do feel considerable concern with your idea of using the exception_ptr
> >> as a payload for the error code. I appreciate the use case for the
> >> Filesystem TS, but I feel the fact that the Filesystem TS returns more
> >> information via the exception throw mechanism than via the error_code
> >> mechanism to be an awful mistake. Some other mechanism should have been
> >> chosen to make both of equal information returning value instead of
> >> splitting the two and making the error code returning overloads inferior
> >> like that. After all, the latter are the natural fit for a Filesystem
> >> library. The former are inappropriately heavyweight.
> >>
> >> What I could live with though is this synopsis:
> >>
> >> template<class T, class P = shared_ptr<void>, class EC = error_code,
> >> class E = exception_ptr> class outcome
> >> {
> >>   T _value;
> >>   EC _error;
> >>   union
> >>   {
> >>     P _payload;
> >>     E _exception;
> >>   };
> >> };
> >>
> >> I personally really wish that shared_ptr could be constructible from an
> >> exception_ptr as they surely are implemented the same way on any
> >> standard library implementation I could think of, but at least the above
> >> correctly lets payload be any type-erased shared_ptr, which is the
> >> correct and appropriate type for returning type-erased payload unlike
> >> exception_ptr.
> >>
> >> The question now becomes this: surely the Filesystem TS is cleaner if
> >> when returning an errored outcome it supplied a shared_ptr<pair<path,
> >> path>> instead of the exception_ptr to the filesystem_error that would
> >> have been thrown? Advantages:
> >>
> >> 1. I would also be fairly sure, without having benchmarked it, that
> >> make_shared<pair<path, path>>(path1, path2) will be many times faster
> >> than make_exception_ptr(filesystem_error(path1, path, ec)) on most
> >> standard library implementations.
> >>
> >> 2. make_exception_ptr(filesystem_error(path1, path, ec)) will return a
> >> null pointer if C++ exceptions are disabled on at least libstdc++, and
> >> thus your proposal would be a problem for those users running with
> >> exceptions off.
> >>
> >> What do you think?
> >>
> >
> > I will not address your question directly; but let me offer a remark.
> >
> > It was my understanding that `error_code_extended`, along with its ring
> > buffer usage, was provided to address exactly this problem: provide
> > additional payload to `std::error_code`. The current exercise with
> adapting
> > the Filesystem TS can be considered a test of the usefulness of
> > `error_code_extended`.
>
> With result<T> which is T|error_code_extended, then yes
> error_code_extended can be used to optionally transport a *fixed* set of
> payload. By its nature, you get storage for one short string currently a
> maximum of 191 characters.
>
> With outcome<T>, we are storing an exception_ptr anyway in this proposed
> simplified non-variant design, so I am suggesting go ahead and union
> that with a shared_ptr, that way you can optionally supply arbitrary
> type-erased payload.
>
> > Some questions arise that i will surely ask in the second review of
> > Boost.Outcome (I hope we will have one):
> >
> > 1. Can `result` be used to successfully replace error handling in
> > Filesystem TS?
>
> Not entirely. outcome<T> as proposed above can. The problem is the *two*
> paths which Filesystem may return as part of filesystem_error. And each
> of those paths could be 16Kb or more.
>
> > 2. If I carry arbitrary payload with a `result` do I need the ring buffer
> > for anything?
>
> Sorry, this misunderstanding is the fault of me cutting out some
> exposition. Let's do it out in its entirety:
>
> ```
> template<class T, class EC = error_code_extended> class result
> {
>   T _value;
>   EC _error;
> };
>
> template<class T, class P = shared_ptr<void>, class EC =
> error_code_extended, class E = exception_ptr>
> class outcome : public result<T, EC>
> {
>   union
>   {
>     P _payload;
>     E _exception;
>   };
> };
> ```
>
> So result<T> retains its trivial copy, move and destruction which is
> important for optimisation. outcome<T> always forces observable
> behaviour because of the potential atomics in exception_ptr, so we lose
> nothing by potentially making the same storage alternatively a shared_ptr.
>
> As mentioned above, the shared_ptr possibility means the Filesystem TS
> now works equally well with C++ exceptions disabled if that TS used
> outcome<T>.
>
> Thoughts?
>

Let's see if I got it correctly.

I, the user of your library, have to make the choice:
1. Either I get minimum-overhead (trivially-copyable) `result<T>`, with
limited ability to carry payload.
2. Or I get ability to carry any payload, but at the expense of paying the
cost of copying `outcome<T>`s.

Since filesystem_error already has non-trivial copy, I loose nothing when
going with option 2 (using `outcome<T>`).

Right?