[boost] Mini-review of LEAF (was: Re: Requesting formal review for (Boost) LEAF)

Subject: [boost] Mini-review of LEAF (was: Re: Requesting formal review for (Boost) LEAF)
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2019-01-05 00:55:15

> I've included in the documentation what I believe is a fair
> analysis of the different design choices made by LEAF and Boost Outcome:
> https://zajo.github.io/leaf/#_comparison_to_boost_outcome.

Firstly, I like this library much more than your preceding design. Well
done on this design!

Secondly, thanks for the fair comparison. I've added an issue to do the
same from the opposite perspective at
https://github.com/ned14/outcome/issues/163.

(Minor niggle: `EP` refers to an "exception" object, not a "pointer"
object. It doesn't need to be a pointer)

Other differences not in your comparison:

1. Outcome doesn't do matchers because I strongly think a separate Boost
library should implement them in a generic way. I would urge you to
split your matchers from LEAF into a standalone library. I think such a
thing would be an *amazing* standalone library, and if you add it,
Outcome will reuse it.

And for the record, lovely matcher design. I think probably optimal
given the current language.

2. Outcome pushes the choice of how to do type erasure and polymorphic
matching onto the end user through their choice of a common E type.
std::error is the poor man's implementation, but probably sufficient for
most users. P1028 https://github.com/ned14/status-code is markedly superior.

LEAF implements in-house what Outcome outsources. I'm not convinced of
the value add here. If you split off the matchers into a standalone
library, and Outcome adds support for those, the difference remaining
between LEAF and Outcome turns into one of how parameters are in the
result<> type. As Outcome using code will almost certainly be typedefing
the underlying type into something more readable, I find the remaining
difference unconvincing of a separate library personally.

3. I get that by not specifying error types in the type one causes
interoperability between disparate libraries to appear to improve in
that everything compiles. But you've really just hidden the potential
misoperation by turning it into a runtime problem.

I deliberately didn't choose that for Outcome for the same reasons that
dynamic exception specifications are a bad idea. I personally think
LEAF's choice on this too close to that terrible design mistake, now
thankfully deprecated from the standard.

You probably now have a gist of my review if your library came to
review. I do love the matchers. Great job. I like most of the rest of
the design, apart from the hiding of ABI incompatibilities into the
(unpredictable) runtime. But I also recognise that the same hiding of
incompatibility affects std::error, and P1028 status code. I just think
that by allowing the user to choose an E type not affected by these
issues, one offers the option to users to avoid that devil's nest.
Meanwhile LEAF *builds in* that devil's nest as a design feature. I find
that problematic.

Otherwise great job, and much improved on before. Thanks for the library!

(And FYI, Herb is not against static exception specifications, your
Design Rationale makes it seem he is. In fact, his main opposition to
them was that the committee would never agree to them due to them
looking too much like dynamic exception specifications. However we got
WG14 to agree to them in the Pittsburgh meeting, and that seems to have
had a big effect on WG21's opinion. But WG21 has yet to debate them,
maybe at Kona, definitely at Cologne. Still, I personally wouldn't claim
what you do in your docs, it's too strong, the guy who has been working
on exceptions support for C for the past thirty years agreed to static
exception specifications. And he *really* knows his stuff on this - C
hasn't added exceptions because the design isn't fully baked yet, not
because they haven't been actively worked upon for decades now)

(Second aside: you appear to assume that thread local storage does not
dynamically allocate memory. The standard specifically allows
implementations to do this, even for a stupid type like int. I think you
can make the claim that no dynamic memory allocation occurs for TLS only
on named architectures and compilers. GPUs currently do not implement
TLS. I think you need to mention that more loudly, as native C++ on GPUs
is becoming real world use case)

Niall