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

Subject: Re: [boost] Outcome/expected/etc/etc/etc
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-06-05 10:48:18

On 05/06/2017 05:43, Gottlob Frege via Boost wrote:
> I've now read ALL the emails.  Here's some thoughts, in no particular order.

Firstly, thank you Tony for wading through all ~750 emails. A lot of
people couldn't find the time, but I think that for those who did it was
a very worthwhile discussion.

I'm currently mostly focused on my final maths exam on Friday - that
being the same degree I was studying whilst with you at BlackBerry that
meant I started a few months late - so my comments below are more off
the top of my head than anything. Still, I've shifted position from
where I was at, and I think I mostly have Robert and Gavin to thank for it.

> - I wonder, Niall, if your uses of the empty state could/should be
> compared to how people sometimes use -1 or 0, etc - ie "special values
> as errors".  I typically don't like that pattern (yet I've done it
> anyhow - performance/laziness/etc - but typically only in localized
> cases, not in "APIs").  So I'm wary about this empty state thing.
> 
> If we do have an empty state, I like it as a separate type - code
> needs to deal with it separately.  I like the idea of taking a
> possibly-empty type, checking for empty, and creating a never-empty
> type, and passing that along so future code need not worry.

My view on that is:

- Use the non-empty type for function returns.
- Maybe use the empty type locally as a local optimisation.
- Internal code hidden away in detail can do what it likes as it always
does anyway.

> - I think the narrow and wide contracts should be in the same type,
> not separate types.  This is C++.
> I suggest "access" if you want long names.  ie
>      T & val = oc.access_value(); // possible UB
> 
> (I almost also suggested "deref" but "deref_value" is wrong - you are
> deref'ing the outcome, not the value)

Nice name choice.

As I've mentioned, the wide-by-default observers will have narrow
editions too, now quite likely prefixed by .access_*(). But I think
there remains use for a dedicated type which is all narrow - for
example, imagine a test suite which typedefs in the narrow edition and
runs the code through static analysis and makes debug binaries with the
sanitisers enabled, and then typedefs in the wide edition for production
binaries.

I'm not saying that will suit all, but it is a valid use case.

> - I agree with Vicente's concern that functions that behave
> differently need to be named differently.  I haven't looked at the new
> (or old) API carefully enough, but as a (possibly hypothetical)
> example:
>      wide_type::value() // returns value, or throw if no value
>      narrow_type::value() // returns value, or UB if no value
> These functions are not the same, and should not be named the same.
> Since you thus need different names anyhow (ie value and
> access_value), why not put them both on the same API?
> I'm not yet sure if you end up with similar examples with
> possibly_empty_type::something() and never_empty_type::something().

I'm not as bothered about this as some people. From my perspective,
different types are different types, .get() in one is allowed to be
subtly different from .get() in another.

> - I see value in the idea that the narrow contract takes more typing,
> but I actually *like* using * and -> for the narrow contract because I
> have 30+ years of training in looking at * and -> and recognizing it
> as "this may be unsafe, examine the code to see how it is guaranteed".
> Boost.Optional specifically chose * for that very reason.

My big objection was, and still is, and will continue to be that if you
are not fulfilling the pointer/iterator contract, stop behaving
partially and brokenly somewhat like a pointer/iterator. Optional is not
a pointer, cannot act like a pointer, and making operator*() available
on it encourages people to do (*o) all the time instead of binding an
lvalue ref to its .value() which is far safer, more explicit as to
intent, and better coding.

> - exception() returns error, error() doesn't return exception, etc:
> would it be better/easier/clearer if we introduced another name, such
> as "failure" or whatever?  ie exception() only returns the stored
> exception, error() only returns the stored error(), and then
> has_failure() would return true in either case, failure_exception()
> returns exception, generates an exception from error if necessary,
> etc?

A number of people have suggested this. I've not been persuaded
personally, in Outcome .has_exception() already returns true if error or
exception. In Outcome there is always a linear flow of less to greater
representation throughout the design, so you can always use the most
representative type/observer and you are *guaranteed* to never lose
information with the minimum possible boilerplate.

Now, some may say that calling .exception() on an errored object is
wasteful because you must silently construct an exception_ptr for a
system_error. But equally, I'd counter that if you actually care about
that, check .has_error() and yank out the error_code before calling
.exception().

But I'll come back to this point later down below.

> - I dislike "exceptions are for exceptional circumstances".  Yes it is
> a common thing, but it is usually wrong.  Using Outcome to enable the
> pattern of "handle errors, but totally cancel the operation on
> exceptions" might actually turn out to be a useful pattern, but I
> think this is actually slightly different from the "exceptions are for
> exceptional cases" camp, as they typically use that chant to choose
> whether their API will use exceptions or errors, not both in a single
> API.
> So I agree with Emil - for most APIs, error-codes vs exceptions" is
> basically trying to handle the same thing - failure (or
> "disappointment").

This certainly is not the case in AFIO v2, nor any of the stuff built on
top. There expected failure is something handled at the point of
failure. Unexpected failure means abort the current operation entirely.
The upcalling of expected failure into unexpected-please-abort is common
enough too, for example in AFIO's byte range locks if the unlock
operation fails after a corresponding lock, we convert that into an
abort because that probably means a network drive has vanished and there
is no point doing anything more now.

> - this ring buffer thing sounds interesting.  I haven't looked at it
> at all, so maybe this makes no sense: should it use thread-local?

Alas passing error_code instances between threads is extremely common,
and we want to keep error_code_extended's move constructor trivial.

> - I think std::expected should always throw something deriving from
> std::exception.  So if E derives from std::exception, we can throw it.
> If E is exception_ptr we can (re)throw it. If E is
> error_code/system_error/etc we can figure out what to throw.  If E is
> user-type, then we wrap it in bad_expected or whatever. (And/or allow
> user E-to-exception customization somehow).  This sounds complicated,
> but it is easily summarized/taught as "the standard always throws
> std::exceptions"

It should be born in mind that all these added layers you suggest, and
indeed Peter has been adding to his implementation, well both me and
Vicente have been down this road already. Original Expected had lots of
complicated special carve outs for certain types in T and/or E. Outcome
in the past had more special carve outs too.

But I'll come back to this below.

> -   I HATE the dual interface of std::filesystem.  Worse, there is a
> chance it is setting a precedent for future std APIs (dual APIs have
> been proposed elsewhere).  Let's please kill it before a dual API
> strikes again.

Networking TS is in some ways worse rather than better because it will
supply failures to you in an error_code, but not let you supply failures
to it the same way :(.

I always wished that ASIO made the const error_code& passed in
non-const, and you could set it on return from the handler. You can
still throw an exception of course to abort everything, but sometimes
you don't want to abort everything. Sometimes you just want to fail.

> However, the other problem with trying to "single-fy" the
> std::filesystem (or any other) API is that users who want error codes
> sometimes don't want the cost of the extra data (paths, etc).  The
> only way to handle that may be with customizable callback functions
> that capture the error-info however you like, or something similar to
> callbacks.

If the function is inlined, the compiler is very clever at totally
eliding returning anything unused. It doesn't even compute the value.

>  - I really look forward to Peter's never-empty variant - and have
> been hoping for one ever since std::variant was finalized. Not because
> I dislike std::variant, but just because I think the option should be
> there for those who need/want it.
> Regardless, std::variant, in my world, is never empty, since it is
> only empty when a move operator throws, which should be never and/or
> when I can't allocate 64 bytes, in which case I'm screwed anyhow.
> Some days I'm like "man just accept empty, it would be a simple API",
> but then I think "it is stupid for variant<int, double> to be empty".
> Other days I think "just double buffer when necessary" (I assume
> that's your direction Peter?), but then I think "I don't want
> double-buffering variant<list, vector> to go on/off based on whether I
> use MS std vs libc++ etc" and also "I don't want double buffering for
> cases that only happen in theory, not in practice".  I like the
> trade-offs of std::variant.
> I do agree that Boost should have a variant2 with different choices
> than std::variant - I think that actually helps the standard - we can
> choose a direction and instead of telling people "tough luck" when
> they disagree, we can say "use boost variant2 when you want those
> properties".

Personally speaking, I think the valueless-by-exception state should
remain possible if the user feeds std::variant more than one type with a
throwing move constructor. For all other cases, you get guaranteed never
empty.

I think that a reasonable balance. Using std::variant already adds
significantly to compile times, sufficiently so I would be cautious of
**ever** including that header in another header file. Adding double
buffering would make compile time load even worse.

And besides, if the user is foolish enough to feed std::variant types
with throwing move constructors, they deserve what they get!

> - I agree with those who were saying (or thinking) that Boost was -
> and should again be - the place where APIs go before going into the
> standard.  APIs (mostly) shouldn't bypass boost and go to the standard
> directly.  Or at least an API needs a similar user-base (both in scale
> and diversity - ie an internal google API has scale, but not
> necessarily diversity).
> Since I know a number of LEWG members agree with me on this, don't be
> surprised to see some push-back from the committee for proposals
> without that kind of backing, and also don't be surprised if Boost
> gets flooded with "the committee told me to come here first".
> Of course I don't speak for LEWG, so maybe none of that happens, we
> haven't had that discussion yet.

Me personally, I have LONG had serious concerns about authors skipping
Boost and going straight to LEWG. I have been banging those concerns for
years now. People weren't listening until very recently.

What I would also say is that LEWG authors do NOT need to commit to a
full Boost library with all that entails. The two most important things
they need to do is a peer review and get a reasonable number of users,
preferably > 100. That may lead naturally to a Boost library, but it
also may not because some libraries are unsuitable for Boost.

What I mean is that in the end Boost cannot accept a library like
Outcome where a lack of consensus exists about what form such a thing
should take. But the peer review was extremely valuable, and the
publicity generated brings in users. So from the point of view of LEWG,
all boxes are ticked, even if no Boost library results from the
submitted library. In other words, **successful rejection** is also a
very high value outcome with large benefits to the wider C++ ecosystem.

> I actually probably had more to say - I should have made notes as I
> was reading all... those... emails.

Right, so on to where I am currently at regarding Outcome's future. Some
may have noticed I have been experimenting with a C++ 17 std::variant
based Outcome, but I have been becoming increasingly dismayed with the
compile time costs, and that's with a far from complete implementation.
This is why presented Outcome does so many tricks with the preprocessor,
it was to reduce compile time load to something reasonable. Without
those tricks we are back to where I was before where I think it
unreasonable to ask users with large code bases to use these objects in
their public APIs :(

Robert's and Gavin's thoughts on a much simpler (i.e. non-variant,
non-constexpr) design and implementation have found surprising resonance
with me in the days since the review ended. After all:

sizeof(error_code_extended) = 24
sizeof(exception_ptr) = 8

So for a type predominantly used solely for returning results from
functions where you will only ever be returning one of them at a time,
there is a very strong argument in favour of ditching the variant
storage in exchange for considerable improvements in:

- implementation complexity
- compile times for end users
- load on the compiler's optimiser

... for the hardcoded EC = error_code_extended and E = exception_ptr
Outcome. 32 bytes of overhead over sizeof(T) I suspect will be
unmeasurable statistically (though I will benchmark before deciding).
Plus, class outcome<T> can subclass class result<T> which in turn will
subclass std::optional<T> or T depending on whether it is empty-capable
or not. We would retain a variant-like construction API. I like that
simplicity.

If I go down this route, you would get all narrow observers because that
makes much more sense here - error_code_extended and exception_ptr
default construct to a null object, so just return those directly. Here
a .failure_*() observer family which synthesise failures when needed
would also make lots of sense.

If I do take that route, I think I would drop any Expected
implementation in Outcome, any constexpr programming support, and any
monadic programming support, and with that I can return to C++ 11 only.
I can already see Expected is going to become expected<T, E1, E2 ...>
and it makes no sense to not use std::variant<...> to implement that
(assuming future compilers very considerably improve its compilation
speed), and I also think Peter is well ahead of everybody including
myself on implementing one of those. Better then to provide explicit
conversion to/from any arbitrary expected<T, E...> implementation from a
stupidly-simple-Outcome, as Vicente suggested. Let Outcome specialise on
its niche, and leave the generalisation to other libraries.

As I said, I have other stuff on my plate right now, and so these are
just vague thoughts plucked from the air. But I am minded that Outcome
closely duplicating and reimplementing variant2 makes no sense, and
besides, **I have no personal need** for an Expected implementation nor
indeed any variant storage. For me, simpler implementation = fewer weird
corner cases = less maintenance. That's lots of boxes ticked in my book.

Niall

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