Re: [review][LEAF] Review of LEAF starts today : May 22 - May 31

From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2020-05-30 23:18:01

On Sat, May 30, 2020 at 11:18 AM Bjorn Reese via Boost <
boost_at_[hidden]> wrote:
> There is runtime overhead in creating and destroying the error contexts
> even when no failure occurs. Whether or not that overhead is acceptable
> depends on circumstances. It seems to me that the noexcept performance
> overhead in the successful case and the failure case are not that
> different (assuming the error payload is not too heavy), so it may be a
> good idea to choose LEAF when worst-case performance is more important
> than average-case performance.

I wrote a benchmark. It's worth mentioning that when used without exception
handling, LEAF does not depend on aggressive RVO optimizations to be able
to communicate errors efficiently, since they are not contained in the
return object. They skip the stack frames and go straight to the error
handler.

> The more I experimented with LEAF, the more I wished that there was an
> error handing API with less syntatic sugar.
>
>    1. Single-stepping into a TryBlock is really cumbersome. The developer
>       has to single-step through layers of unfamiliar templated code
>       before reaching their own TryBlock. This interrrupts their flow.
>       Setting a breakpoint in the TryBlock helps but is still cumbersome.

Yes. You can set breakpoints at these two lines to stop just before an
error handler is called:

https://github.com/zajo/leaf/blob/develop/include/boost/leaf/handle_error.hpp?ts=4#L611
https://github.com/zajo/leaf/blob/develop/include/boost/leaf/handle_error.hpp?ts=4#L623

>    2. Some compilation errors are difficult to interpret. For example, if
>       you accidentally omit a required return statement in one of the
>       try_handle_all handlers, then the compiler reports the start of the
>       try_handle_all statement and some internal LEAF voodoo. If you
>       are experienced with reading templates errors, then you may be able
>       to discover the erroneous handler from the LEAF template parameters
>       in the compiler output.

Also yes. I'm all ears if you have suggestions. This stuff is tricky, e.g.
consider that when using try_handle_some, if you're returning a
result<void>, it would be unfortunate to require a return { } from the
error handlers (in case of success). LEAF does not require it, but there's
code to deal with that and maybe a few other similar voodoo.

> The LEAF handling is reminiscient of a variant visitor, but with the
> variants we can use index() and std::get<T> in a switch statement
> instead of visitation. I would like to see something similar for LEAF
> error contexts.

The difficulty with that is, context<E...> requires a list of types so it
knows what to put in the tuple. In other words, before you are able to say
ctx.get<T>(), you must have included that T in the types used to
instantiate context<E...>, and if you didn't, you'd always get a null.
Worse, if you missed to include a T in the list of types, and an
error-reporting function attempts to communicate a T, it'll get discarded
(no storage for it). That's why LEAF deduces the E... type list from the
types that error handlers take as arguments. This way, if a T is discarded,
it means no error handlers needed it, and so it'd be wasteful to spend
cycles transporting it.

> The remote_handle_all() name is confusing. I would rename it to
> nested_handle_all().

That would be misleading because it may not be nested (English is not my
first language). I've spent a long time trying to come up with this name, I
do want to rename it if a better name is suggested.

> std::ostream is entangled into various classes for printing. I would
> prefer if printing was moved into seperate X_io.hpp files to speed up
> compilation times when not using them.

Not possible because printing is optional. LEAF will bind it if available,
but if not, you won't get a compile error. This is the same as in Boost
Exception.

> The LEAF_CONSTEXPR should be renamed to LEAF_CXX14_CONSTEXPR to align
> with the BOOST_CXX14_CONSTEXPR naming. Also the #if should use '>='
> instead of '>'.

Yes, the constexpr stuff needs to be cleaned up and made perfectly correct.

> >    - What is your evaluation of the potential usefulness of the library?
>
> LEAF presents a unique error handling solution, that may be useful to
> certain users.

In a way it isn't so strange. It's as if catch() could take multiple
arguments that are matched by their static, rather than dynamic type.

Thanks once again!