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

classic Classic list List threaded Threaded
54 messages Options
123
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
The Boost formal review of Emil Dotchevski's LEAF (Lightweight Error
Augmentation Framework) library will take place from May 22 through May
31st.

LEAF isn't just another error reporting library in the family of
expected-like types. It provides a unique take on error handling which
plays into usability, flexibility, and performance.

Every software developer must deal with handling errors which makes this
review relevant to all. Please participate! I will be grateful to
receive a review based on whatever level of effort or time you can devote.


LEAF is brought to us by Emil Dotchevski, the author of Boost.Exception.
Similar to Boost.Exception, this library allows arbitrary error objects
to be returned; however, unlike Boost.Exception it does not require
dynamic memory. The library can be used with or without exception handling.

The LEAF documentation highlights the following features:

* Efficient delivery of arbitrary error objects to the correct error
  handling scope.
* No dynamic memory allocations.
* Compatible with std::error_code, errno and any other error code type.
* Can be used with or without exception handling.
* Support for multi-thread programming.

"LEAF is designed with a strong bias towards the common use case where
callers of functions which may fail check for success and forward errors
up the call chain but do not handle them."

LEAF offers a pattern matching facility to specify which errors to
handle and how to handle them. Take a look at the 5-minute Introduction.
<https://zajo.github.io/leaf/#introduction>


You can find the source code to be reviewed here:
  <https://github.com/zajo/leaf/tree/review>

You can find the documentation here:
  <https://zajo.github.io/leaf/>

The documentation includes a tutorial, examples, reference, design
rationale, and comparisons to other error handling approaches/libraries.

There is a benchmark document also:
  <https://github.com/zajo/leaf/blob/master/benchmark/benchmark.md>

and a whitepaper:
  <https://github.com/zajo/leaf/blob/master/doc/whitepaper.md>


Please provide in your review information you think is valuable to
understand your choice to ACCEPT or REJECT including LEAF as a
Boost library. Please be explicit about your decision (ACCEPT or REJECT).

Some other questions you might want to consider answering:

  - What is your evaluation of the design?
  - What is your evaluation of the implementation?
  - What is your evaluation of the documentation?
  - What is your evaluation of the potential usefulness of the library?
  - Did you try to use the library? With which compiler(s)? Did you
    have any problems?
  - How much effort did you put into your evaluation? A glance? A quick
    reading? In-depth study?
  - Are you knowledgeable about the problem domain?

More information about the Boost Formal Review Process can be found
here: <http://www.boost.org/community/reviews.html>

Thank you for your effort in the Boost community.

Happy coding -
michael

--
Michael Caisse
Ciere Consulting
ciere.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
> LEAF is brought to us by Emil Dotchevski, the author of Boost.Exception.
> Similar to Boost.Exception, this library allows arbitrary error objects
> to be returned; however, unlike Boost.Exception it does not require
> dynamic memory. The library can be used with or without exception handling.
>
>
This response is in the form of exploratory questions to the author, which
I hope will aid my evaluation.

Disclaimer: At present I am one of the few people in the world who uses
nested exception handling (similar to boost.exception) in production
programs for reasons of:
- efficiency in the non-exception case
- readability (not mixing error handling concerns with logic concerns)
- ambivalence about the cost of memory allocation or dynamic lookup in the
rare case of an exception being thrown
- collection of all data that led to the cause of the exception, without
having to collect a stack trace

Questions are being asked from the basis of deciding whether there would be
an advantage in switching to LEAF.

The LEAF documentation highlights the following features:
>
> Some other questions you might want to consider answering:
>
>   - What is your evaluation of the design?
>

Early to say, but I have some questions.

- From looking at the tutorial it appears that in order to use this library
I would need to write functions that are interested in catching errors in
terms of a group of lambdas. Is this correct in general, or is this merely
an artefact of the example code?

- Furthermore, it seems that any function which may produce an error
(previously an exception) or pass one down the caller chain, would need to
be rewritten in terms of the LEAF macros and return types. Is this correct,
or have I misunderstood?

- It would be interesting to me to see a real program, that sees production
use, written in terms of LEAF in order to assess the maintenance and
readability impact of this. Is there an example of one available?

- Does LEAF incur any memory or runtime performance overhead in the
non-error case? If so what is the impact of this when compared to the
itanium zero-cost exception handling ABI?


>   - What is your evaluation of the implementation?
>

Not yet ascertained.


>   - What is your evaluation of the documentation?
>

Clear, friendly and complete.


>   - What is your evaluation of the potential usefulness of the library?
>

TBA


>   - Did you try to use the library? With which compiler(s)? Did you
>     have any problems?
>

No


>   - How much effort did you put into your evaluation? A glance? A quick
>     reading? In-depth study?
>

Detailed reading of the tutorial, careful consideration of what I perceive
to be the impact of adoption based on that reading.


>   - Are you knowledgeable about the problem domain?
>

Yes


>
> More information about the Boost Formal Review Process can be found
> here: <http://www.boost.org/community/reviews.html>
>
> Thank you for your effort in the Boost community.
>
> Happy coding -
> michael
>
> --
> Michael Caisse
> Ciere Consulting
> ciere.com
>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>


--
Richard Hodges
[hidden email]
office: +442032898513
home: +376841522
mobile: +376380212

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost <
[hidden email]> wrote:

> - From looking at the tutorial it appears that in order to use this library
> I would need to write functions that are interested in catching errors in
> terms of a group of lambdas. Is this correct in general, or is this merely
> an artefact of the example code?
>

The lambdas aren't so much to catch errors, but to access additional
information associated with the exception (a-la Boost Exception, but a lot
more efficient and with better syntax). If you just want to catch the
exception, you can do a plain old try...catch (of course).


> - Furthermore, it seems that any function which may produce an error
> (previously an exception) or pass one down the caller chain, would need to
> be rewritten in terms of the LEAF macros and return types. Is this correct,
> or have I misunderstood?
>

You misunderstood. If you use exceptions, you don't need a result<T> type
or LEAF_AUTO. You probably got confused because the initial introduction
starts with a result<T> example, but it continues with the same example
using exceptions:

With exceptions: https://zajo.github.io/leaf/#introduction-eh

With result<T>: https://zajo.github.io/leaf/#introduction-result


> - It would be interesting to me to see a real program, that sees production
> use, written in terms of LEAF in order to assess the maintenance and
> readability impact of this. Is there an example of one available?
>

I do not have open source production code to share, but I could possibly
show you some code personally. You mentioned Boost Exception, perhaps
you'll find this section helpful:
https://zajo.github.io/leaf/#boost_exception. Switching from Boost
Exception to LEAF is straight-forward (and generally recommended).


> - Does LEAF incur any memory or runtime performance overhead in the
> non-error case? If so what is the impact of this when compared to the
> itanium zero-cost exception handling ABI?
>

Generally, no.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
I notice that the LEAF documentation comes with benchmarks comparing
leaf::result to other return-based error handling systems.  That's good.

But I also notice that it doesn't come with benchmarks comparing
exception-based LEAF with other exception-based error handling systems
like Boost.Exception.  Nor does it come with benchmarks comparing
leaf::result with leaf::exception.  That's less good.

I am currently using Boost.Exception, and I am satisfied with its
functionality, but I am worried about its performance cost.  LEAF claims
to be a faster functional replacement for Boost.Exception, and is for
this reason seems worth investigating.  However, this claim does not
appear to be backed up by any hard numbers.


--
Rainer Deyke ([hidden email])


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
On Sun, May 24, 2020 at 1:14 AM Rainer Deyke via Boost <
[hidden email]> wrote:
>
> I notice that the LEAF documentation comes with benchmarks comparing
> leaf::result to other return-based error handling systems.  That's good.

By the way, to use LEAF without exceptions, you don't have to use
leaf::result. For example, if you have a program that generally
communicates failures in terms of some other result<T>, usually you need
not change that in order to utilize LEAF to transport additional error
objects when needed.

As for the benchmark, it is not very fair to LEAF: it only tests the
performance when transporting a single error object. The killer feature of
LEAF is the ability to efficiently associate with a single failure any
number of error objects, of arbitrary types. To do this with another result
type, you'd have to allocate memory dynamically.

> I am currently using Boost.Exception, and I am satisfied with its
> functionality, but I am worried about its performance cost.  LEAF claims
> to be a faster functional replacement for Boost.Exception, and is for
> this reason seems worth investigating.  However, this claim does not
> appear to be backed up by any hard numbers.

Thank you for using Boost Exception. The claim that LEAF is faster is based
on the fact that Boost Exception allocates error objects dynamically and
keeps them organized in a std::map (stored in the exception object itself),
while LEAF keeps all error objects in a std::tuple stored in the stack
frame of the error-handling function that needs them (e.g. where the
try...catch is).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Thank you for your response.

On Fri, 22 May 2020 at 21:30, Emil Dotchevski via Boost <
[hidden email]> wrote:

> On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost <
> [hidden email]> wrote:
>
> > - From looking at the tutorial it appears that in order to use this
> library
> > I would need to write functions that are interested in catching errors in
> > terms of a group of lambdas. Is this correct in general, or is this
> merely
> > an artefact of the example code?
> >
>
> The lambdas aren't so much to catch errors, but to access additional
> information associated with the exception (a-la Boost Exception, but a lot
> more efficient and with better syntax). If you just want to catch the
> exception, you can do a plain old try...catch (of course).
>
> and
> - Does LEAF incur any memory or runtime performance overhead in the
> non-error case?

I think I understand now. The construct of try_handle_all(lambdas...)  is
building an "error context" into which later leaf-enabled functions down
the call stack can emplace error info. The address of this context is
maintained by logically a separate "context stack" which lives in the call
stack. To be strictly correct, would it be fair to say that there is memory
allocation, it just happens to be on the call stack and therefore requires
very little book-keeping overhead and no mutual exclusion?

Additionally, a small (presumably small) deterministic setup cost is
incurred whether or not error information will be produced.

Would that be accurate?


> I do not have open source production code to share, but I could possibly
> show you some code personally.


This would be helpful.

At the moment, I get the impression that this is a very elegant solution to
a problem I don't have. I want to be even handed during the review. It
would be helpful to see some code from a domain where this solution solves
a very real problem, or have a domain described where LEAF clearly solves a
real problem. For that "Oh Wow!" moment that you have presumably personally
experienced.



>
> --
Richard Hodges
[hidden email]
office: +442032898513
home: +376841522
mobile: +376380212

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Disclaimer: This is from someone not using Boost.Exception, Expected or
any other result<T> library and judging from the documentation. So this
might serve in evaluating how well the lib/documentation fares to
someone thinking to adopt this.
>    - What is your evaluation of the design?
There is a mixture of lambdas and macros which I find a bit off-putting
on first sight. However it seems they solve the problem well in the
constraints of the language.

I find the default for `is_e_type` being true for any X having X::value
a bit broad. This basically applies to all strong types so it feels like
an "E-Type" could be anything. So I don't understand why the distinction
into E-types and non-E-types is made.

Maybe improve on `remote_handle_all` in 2 ways: Create the lambda taking
`leaf::error_info const&` in that function to remove the boilerplate at
the user call site. Given that rename it to e.g. `create_remote_handler`
or so. I find the current interface confusing and the documentation
calls it "unintuitive", so I guess this should be improved on before
release. Given that: It seems `remote_try_handle_all` can be folded into
the regular `try_handle_all`. So `try_handle_all` takes a list of
handlers where one could be the aggregated handler defined by
`create_remote_handler` (then maybe `create_aggregate_handler`?)

I'm interested in knowing how `try_handle_all` implements "if the user
fails to supply at least one handler that will match any error, the
result is a compile error". If arbitrary functions can be called
throwing any error type, how could that work? Is it again required to
use a catch-all clause?

>    - What is your evaluation of the documentation?
Good overall, but the details are to detailed for an introduction and to
coarse for understanding it.
Examples by header:
- "Using Exception Handling": virtual inheritance is mentioned but the
reasoning is not clear to me. I have never used or seen it used in that
context and always strive to avoid it due to the issues with handling
such an object (starting at writing ctors)
- "Accumulation": shows how to call leaf::accumulate but I don't see
what it use is. "Many different files" are mentioned but the operation
only takes a single file. And based on everything before a single error
stops execution (like exceptions)
- "E-types": I didn't got what an E-type is exactly. It starts with
"values" you can add to "errors" or "exceptions" but seemingly
exceptions itself are "E-types"? See also above. I also didn't see the
enums being mentioned here that are used in other places. This would be
a great addition
- "Loading": It starts how values are put into a `context`. I feel that
a detailed introduction how a context works and is used/accessed prior
to this would be useful. My point is: The `leaf::load` call is somewhere
down the chain not knowing anything about the `try_handle_*` call and
hence the context. How does it know about it? Or what to put there? So
maybe start a first paragraph solely on the context and its access
- "error_id": It is unclear to me what it's use is. Especially as
something unique in the program in the context of threads means some
kind of synchronization which will be paid for even if unused.
- "defer": I'd like to know when the lambda is not called. The
documentation says it is called in the dtor, but later: "function passed
to defer, if invoked,"
- "Working with Disparate Error Types": To me it is unclear what the
message of this part is. Maybe it can be shortened.
- "Lua": Code example is missing the namespace for `augment_id` and
there is likely a typo "aughent"

Wording at some places might be improved.
- "With LEAF, users can efficiently associate with errors or with
exceptions any number of values that pertain to a failure", for me this
is hard to understand.
- "Let’s say we want to build a record of file locations a given error
passes through on its way to be handled", is something missing here?

As it is a C++11 library I'd suggest to not use plain enums in the
documentation. E.g. `enum do_work_error_code` in the lua example with
such generic names. This might lead beginners into C&P the code.

>    - What is your evaluation of the potential usefulness of the library?
It seems to solve the same problem as other error handling libraries but
in a new way.
Performance-wise it seems to be comparable. I'd suggest to evaluate the
cases where it lacks behind and whether those can be improved

Giving the intention to unify error handling (or try to make it more
uniform) I'm wondering if this could be standardized. Given that Boost
is often a "playground for future C++ standards" I feel this should be
evaluated whether this is possible at all. Of course I'd love to see
language support avoiding lambdas and macros, but I'm not optimistic
regarding that.

I'd also like to see a comparison with exceptions especially in cases
where the usage of result<T> inhibits (N)RVO (even more for C++17 and
up). This is my main critic point about result<T> type of error handling
as that variant type creates less efficient code for the happy path.

All in all I think it is useful to experiment with a new approach and as
such fits Boost

>    - Did you try to use the library? With which compiler(s)? Did you
>      have any problems?
Didn't try
>    - How much effort did you put into your evaluation? A glance? A quick
>      reading? In-depth study?
In-depth reading of documentation only.
>    - Are you knowledgeable about the problem domain?
Not really.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>   - What is your evaluation of the design?

I like it.

>   - What is your evaluation of the implementation?

I like it.

>   - What is your evaluation of the documentation?

Very good.

>   - What is your evaluation of the potential usefulness of the library?

Pros:

1. It is a logical extension of Boost.Exception.

2. It is more useful and better than Boost.Exception.

3. It delivers 99% of C++ exceptions, without requiring C++ exceptions
enabled globally. For those users who work in a C++ exceptions globally
disabled environment, but do have TLS available to them, this solves
that exact audience.

4. It delivers pattern matching catch handlers as a library
implementation, no small feat. We'll be waiting at least until C++ 23
for this to reach the language.


Cons:

1. Me personally, if I want to write success-orientated logic where I
don't write out, inline, what happens for each failure, I just write
code which uses C++ exceptions. I mean, there is a perfectly good
implementation of LEAF already shipping with every C++ compiler for the
last twenty years. It is called "C++ exception handling".

2. I, personally speaking, think that the shops which globally disable
C++ exceptions do so because they specifically wish to ban the use of
C++ exceptions type code in their codebases. Ergo, they would also ban
the use of LEAF, because this exact pattern and style of code is what
they are banning, and globally disabling C++ exceptions is a simple way
of achieving this.

3. For those codebases which really DO want C++ exception handling, but
can't enable it for space or determinacy reasons (i.e. embedded, GPUs),
the reliance on TLS is a showstopper. Back when I designed Outcome, in
order to be as broadly useful as possible, I very deliberately made sure
that the design would work well in (a) consteval evaluation contexts, so
we can implement compile-time error handling using Outcome and (b) in
non-TLS-capable environments such as embedded systems and GPUs.

4. The lack of built-in support for C++ Coroutines I find unfortunate.
Outcome was initially designed to be compatible with C++ Coroutines, but
I never thought it would become a major use case for Outcome back at
that time. It has since become evident to me that C++ Coroutines is a
very major use case for Outcome, because C++ Exceptions and C++
Coroutines are an ugly fit riddled with nasty gotchas, so it's just
easier to make everything noexcept, wrap your coroutine bodies in
try-catch(...) to trap coroutine frame allocation failure, and
exclusively return outcome::result<T> from all your coroutines. Now
that's quite a bit of tedious boilerplate working around what is a
defect in the C++ 20 spec, but it works. But what I think I, and lots of
others, really would prefer instead is for something C++ exceptions like
(e.g. LEAF) to seamlessly work well with C++ Coroutines. I think that if
so retargeted, LEAF would have much broader usefulness.


All the above isn't to say that LEAF isn't useful. It's just not
*broadly* useful, in my opinion. And, to be specific here, the specific
niche where LEAF is most useful (gaming) is, in my opinion, the most
likely to refuse to use it for political/org/cultural reasons.

>   - Did you try to use the library? With which compiler(s)? Did you
>     have any problems?

I extensively tested it more than a year ago. I did not test it since
then. It was absolutely fine a year ago, I do not expect anything except
improvement since then. Emil is an excellent engineer.

>   - How much effort did you put into your evaluation? A glance? A quick
>     reading? In-depth study?

More than a year ago I spent a lot of time studying it, mainly to
benchmark its codegen against Outcome, such that I could make a better
codegen Outcome which is v2.2, expected to be merged to trunk end of
2020. I have done very little since, except a cursory overview for this
review.

>   - Are you knowledgeable about the problem domain?

Yes, intimately.


For me personally this library is a weak accept. I can't personally see
any use cases for it, but that's not to say there aren't some out there
who have this exact problem needing solving, for reasons I cannot think
of. And the implementation is high quality, as are the docs, and it's a
straightforward and logical extension of Boost.Exception which is
already shipping in Boost. And because it's that logical extension, that
makes it an accept for me. I just wish it were more broadly useful,
that's all.

Niall

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
On Fri, May 29, 2020 at 6:20 AM Niall Douglas via Boost
<[hidden email]> wrote:
> >   - What is your evaluation of the design?
> I like it.

I didn't have enough information to form an opinion of LEAF, but
Niall's endorsement definitely sets off warning bells and red flags.

Regards

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Fri, May 29, 2020 at 1:10 AM Richard Hodges via Boost <
[hidden email]> wrote:

>
> Thank you for your response.
>
> On Fri, 22 May 2020 at 21:30, Emil Dotchevski via Boost <
> [hidden email]> wrote:
>
> > On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost <
> > [hidden email]> wrote:
> >
> > > - From looking at the tutorial it appears that in order to use this
> > library
> > > I would need to write functions that are interested in catching
errors in
> > > terms of a group of lambdas. Is this correct in general, or is this
> > merely
> > > an artefact of the example code?
> > >
> >
> > The lambdas aren't so much to catch errors, but to access additional
> > information associated with the exception (a-la Boost Exception, but a
lot

> > more efficient and with better syntax). If you just want to catch the
> > exception, you can do a plain old try...catch (of course).
> >
> > and
> > - Does LEAF incur any memory or runtime performance overhead in the
> > non-error case?
>
> I think I understand now. The construct of try_handle_all(lambdas...)  is
> building an "error context" into which later leaf-enabled functions down
> the call stack can emplace error info. The address of this context is
> maintained by logically a separate "context stack" which lives in the call
> stack. To be strictly correct, would it be fair to say that there is
memory
> allocation, it just happens to be on the call stack and therefore requires
> very little book-keeping overhead and no mutual exclusion?

Yes. I meant no dynamic allocation. The minimal bookkeeping is logically
similar to a tuple<optional<E>...>.

> > I do not have open source production code to share, but I could possibly
> > show you some code personally.
>
> This would be helpful.
>
> At the moment, I get the impression that this is a very elegant solution
to
> a problem I don't have. I want to be even handed during the review. It
> would be helpful to see some code from a domain where this solution solves
> a very real problem, or have a domain described where LEAF clearly solves
a
> real problem. For that "Oh Wow!" moment that you have presumably
personally
> experienced.

If I'm not mistaken you said you use Boost Exception. That indicates you do
have the problem that you said LEAF solves more elegantly (and more
efficiently) than Boost Exception.

To be more precise, LEAF is designed to help you communicate _all_ data
needed to handle a given failure, from any scope it is available, to the
correct error handler. It can do this even through layers of uncooperative
APIs.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Fri, May 29, 2020 at 1:26 AM Alexander Grund via Boost <
[hidden email]> wrote:
> Maybe improve on `remote_handle_all` in 2 ways: Create the lambda taking
> `leaf::error_info const&` in that function to remove the boilerplate at
> the user call site. Given that rename it to e.g. `create_remote_handler`
> or so. I find the current interface confusing and the documentation
> calls it "unintuitive", so I guess this should be improved on before
> release. Given that: It seems `remote_try_handle_all` can be folded into
> the regular `try_handle_all`. So `try_handle_all` takes a list of
> handlers where one could be the aggregated handler defined by
> `create_remote_handler` (then maybe `create_aggregate_handler`?)

I've explored folding it all into try_handle_all, but the implementation
gets really messy. Perhaps it can be done. Consider that in this case the
loop that matches the lambdas based on what error objects are available at
runtime now has to be recursive. Maybe it can be done elegantly, but I
tried and gave up.

> I'm interested in knowing how `try_handle_all` implements "if the user
> fails to supply at least one handler that will match any error, the
> result is a compile error". If arbitrary functions can be called
> throwing any error type, how could that work? Is it again required to
> use a catch-all clause?

Yes, you must supply a handler that matches any error. If you don't want to
do that, use try_handle_some. The benefit of _all is that it unwraps the
result for you: where try_handle_some returns a result<T>, try_handle_all
returns T. It also guarantees at compile time that the user has provided a
suitable handler for all errors.

> >    - What is your evaluation of the documentation?
> Good overall, but the details are to detailed for an introduction and to
> coarse for understanding it.

Thanks for this feedback.

> - "Using Exception Handling": virtual inheritance is mentioned but the
> reasoning is not clear to me. I have never used or seen it used in that
> context and always strive to avoid it due to the issues with handling
> such an object (starting at writing ctors)

This is a somewhat-known problem with exceptions. If you have:

struct A { };
struct B1: A { };
struct B2: A { };
struct C: B1, B2 { }

Now a catch(A &) won't intercept a C because the cast is ambiguous. Using
virtual inheritance eliminates this problem and allows A to be safely used
as a base of anything that should be classified as A.

> - "Accumulation": shows how to call leaf::accumulate but I don't see
> what it use is. "Many different files" are mentioned but the operation
> only takes a single file. And based on everything before a single error
> stops execution (like exceptions)

Normally you give LEAF an object of type T and it stores it away for you.
If you later give it another value of type T, it'll just overwrite the old
one. With accumulate, you get a mutable reference to the stored value (and
it requires T to have a default constructor).

> - "error_id": It is unclear to me what it's use is. Especially as
> something unique in the program in the context of threads means some
> kind of synchronization which will be paid for even if unused.

See https://zajo.github.io/leaf/#tutorial-model

> - "defer": I'd like to know when the lambda is not called. The
> documentation says it is called in the dtor, but later: "function passed
> to defer, if invoked,"

The logic is the same as for preload, which takes your value, and will
later communicate it if an error has occurred, while defer takes your
lambda and will later call it to get a value to communicate, if an error
has occurred.

> I'd also like to see a comparison with exceptions especially in cases
> where the usage of result<T> inhibits (N)RVO (even more for C++17 and
> up). This is my main critic point about result<T> type of error handling
> as that variant type creates less efficient code for the happy path.

Yes, though leaf::result<T>, in case of an error, only transports a single
int, the error_id, which allows for efficient implementation.

That said, you can use pretty much any result type with LEAF, it does not
have to be leaf::result<T>.

Thanks!

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Fri, May 29, 2020 at 6:20 AM Niall Douglas via Boost <
[hidden email]> wrote:
> 1. Me personally, if I want to write success-orientated logic where I
> don't write out, inline, what happens for each failure, I just write
> code which uses C++ exceptions. I mean, there is a perfectly good
> implementation of LEAF already shipping with every C++ compiler for the
> last twenty years. It is called "C++ exception handling".

When used with exceptions, LEAF is "what if catch could be given many
arguments which are matched statically instead of by dynamic_cast?"
Exceptions would have been more useful were they designed this way, for
example they would not need to be allocated dynamically.

> 2. I, personally speaking, think that the shops which globally disable
> C++ exceptions do so because they specifically wish to ban the use of
> C++ exceptions type code in their codebases. Ergo, they would also ban
> the use of LEAF, because this exact pattern and style of code is what
> they are banning, and globally disabling C++ exceptions is a simple way
> of achieving this.

I'm confused, what style and pattern are they banning? How is what they use
instead semantically different?

> 4. The lack of built-in support for C++ Coroutines I find unfortunate.

I intend to work on that. I'm not sure I should. :)

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
- What is your evaluation of the design?

It's very clean. While I'm not a big fan of the macros, I do like how it's
somewhat
reminiscent of vanilla C++ exceptions. Some minor nit-picking here:
trailing underscores
following names that are public doesn't look particularly right; makes it
look like you're using you
aren't supposed to :)

- What is your evaluation of the documentation?

The documentation is great -- in terms of styles it's probably the best
I've seen for a C++ library.
The examples are great and they do a good job of communicating how the
library works by providing
realistic scenarios. It's quite well done.

- What is your evaluation of the potential usefulness of the library?

While this is not something I have found a need for, the motivation for
this libraries existence is weill founded and I can see use cases for it.

- Did you try to use the library? With which compiler(s)? Did you have any
problems?

I did not.

- How much effort did you put into your evaluation? A glance? A quick
reading? In-depth study?

I spent about an hour going through the documentation.

- Are you knowledgeable about the problem domain?

Not really :)

In terms of acceptance, I'm neutral on that front as this is not something
I am well versed in.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
On Fri, May 29, 2020 at 10:27 PM Krystian Stasiowski via Boost <
[hidden email]> wrote:
> trailing underscores
> following names that are public doesn't look particularly right; makes it
> look like you're using you
> aren't supposed to :)

Thanks for the review. Obviously, that's called catch_ because catch is a
keyword. Good news is I think I can get rid of it altogether, and match
exceptions based on a handler taking an argument of a type that derives
from std::exception. So, instead of:

leaf::try_catch(
  []
  {
    ....
  },
  []( catch_<my_exception>, .... )
  {
  } );

You'd just say:

leaf::try_catch(
  []
  {
    ....
  },
  []( my_exception &, .... )
  {
  } );

OTOH, catch_ can be given multiple exception types, and it'll match if the
caught exception is of any one of the specified types.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Krystian Stasiowski wrote:

> Some minor nit-picking here: trailing underscores following names that are
> public doesn't look particularly right; makes it look like you're using
> you aren't supposed to :)

That's standard Boost/MPL convention for identifiers that would otherwise be
keywords, such as int_ or if_.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 2020-05-22 21:30, Emil Dotchevski via Boost wrote:

>> - Does LEAF incur any memory or runtime performance overhead in the
>> non-error case? If so what is the impact of this when compared to the
>> itanium zero-cost exception handling ABI?
>>
>
> Generally, no.

Can you elaborate on this with regards to runtime performance?

In the successful case, the runtime overhead of C++ try-catch blocks is
a jump over the catch block.

With leaf::try_catch() generates more code. Inside the TryBlock we can
see a couple of leaf functions on the call stack between the calling
functions and the TryBlock.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
On Sat, May 30, 2020 at 7:43 AM Bjorn Reese via Boost <[hidden email]>
wrote:

>
> On 2020-05-22 21:30, Emil Dotchevski via Boost wrote:
>
> >> - Does LEAF incur any memory or runtime performance overhead in the
> >> non-error case? If so what is the impact of this when compared to the
> >> itanium zero-cost exception handling ABI?
> >>
> >
> > Generally, no.
>
> Can you elaborate on this with regards to runtime performance?
>
> In the successful case, the runtime overhead of C++ try-catch blocks is
> a jump over the catch block.

By "generally no" I meant that the work is mostly limited to try_catch.

In more details, on the happy path::

- At try_catch: a local tuple<slot<E>...> is initialized, where E... is
basically the argument types of all the error handlers, with duplicates
removed. Each slot<E> zeroes an int, and a TLS pointer for each of E... is
set to point the corresponding slot<E>... . When exiting the try_catch,
this is undone.

- Things like preload cache all their arguments locally, by moving, later
to be discarded if no error has occurred.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 2020-05-22 11:31, Michael Caisse via Boost wrote:

> Please provide in your review information you think is valuable to
> understand your choice to ACCEPT or REJECT including LEAF as a
> Boost library. Please be explicit about your decision (ACCEPT or REJECT).

I vote to conditionally accept LEAF into Boost. LEAF provides a new
perspective on error propagation that is worth exploring in the real
world. The conditions for acceptance are:

   1. Provide a lower-level API for error handling which does not
      necessitate a pattern matching framework. I would prefer an API
      closer to the variant::index() and std::get<I>() combination that
      enables you to write the error handling as switch statements.
      More rationale for this request is given under the design
      evaluation.


>    - What is your evaluation of the design?

LEAF propagates error objects across multiple function calls by letting
the callee store the error payload directly in a stack object, called
the error context, at the caller. A pointer to the error context is
located in thread-local storage. When there are more error contexts on
the call stack, they are daisy chained via pointers in the child error
context.

This is a sound design, which gives LEAF an efficient way of propagating
error payloads then a failure occurs.

I like that the returned error_id can be passed through C code.

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.

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.

   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.

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.

I am undecided about the reliance on thread-local storage. Initially I
thought that we could just replace thread-local storage with a home-
made lookup table using atomics (which are generally available on
OS-less platforms, whereas mutexes etc are not,) but Niall's mention of
consteval made me unsure (damn you Niall ;-) However, I do not see this
as a show-stopper.

The try_handle_all() and try_handle_some() functions have different
return values. While I understand the rationale given in the reference
documentation, it may still be confusing to users if they change from
one to the other. This could possibly be solved by making it more clear
in the documentation.

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


>    - What is your evaluation of the implementation?

The implementation is of high quality, and reasonably readable if you
are comfortable with templates.

I did not do a proper code review, so the following are random notes.

Some classes (leaf::result and the internal optional) uses placement-new
and explicit destructor calls. I suggest that you create your own
construct_at() and destruct_at() which uses the std counterparts when
available, and placement-new / explicit destruction otherwise. This will
make the code more readable.

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.

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 '>'.

config.h includes <Windows.h>. This should be replaced with
Boost.WinAPI.

>    - What is your evaluation of the documentation?

Overall the documentation is well-presented.

The examples reconfigures std::cout to throw exceptions. That obscures
the examples because now the reader has to learn LEAF and how std::cout
exceptions work at the same time. It would be better to move the
std::cout exception stuff into a separate example which is only about
std::cout.


>    - 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. I do not expect a huge user base though, as LEAF users
mainly are going to be projects with requirements not met by the
alternatives.


>    - Did you try to use the library? With which compiler(s)? Did you
>      have any problems?

I experimented with small examples and found no compiler-related issues.


>    - How much effort did you put into your evaluation? A glance? A quick
>      reading? In-depth study?

About 6 hours. Spent most time trying understanding the design than
reviewing the code and documentation.


>    - Are you knowledgeable about the problem domain?

Yes.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/29/20 22:27, Krystian Stasiowski via Boost wrote:
> - What is your evaluation of the design?
>

<snip review>


>
> In terms of acceptance, I'm neutral on that front as this is not something
> I am well versed in.
>

Thank you for the review Krystian.

One way of thinking about your recommendation for acceptance is, in your
opinion from what you have reviewed, do you think this library is ready
for use? Each person doing a review brings different backgrounds and
will review different aspects.

Please feel free to provide a recommendation about acceptance.

Thank you again for your participation.
michael

--
Michael Caisse
Ciere Consulting
ciere.com

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

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

Boost - Dev mailing list
This is my review of LEAF as a long-time user and proponent of Boost:

> Please provide in your review information you think is valuable to
> understand your choice to ACCEPT or REJECT including LEAF as a
> Boost library. Please be explicit about your decision (ACCEPT or REJECT).

ACCEPT

>  - What is your evaluation of the design?

It's similar to many Boost libraries before it in that it's incredibly
rich, nuanced and can
give you the most cryptic and hard-to-decipher compiler errors.

However, the more I used it and become accustomed to its idioms, the more I
found myself
enjoying it.

`preload` is the most simple and elegant abstraction I've seen for
incrementally adding information
to an error context as it travels back up the callstack. I've seen similar
patterns be attempted in Go
and LEAF far and away has the most powerful and type-safe approach.
`preload` should, however,
have a [[nodiscard]] attribute attached to it.

`try_handle_all` is a nicer alternative to exception handling though it can
be difficult to tell if an overload
is going to be called when trying to match up against errors. I'm assuming
this is simply because of my
inexperience with the library and is something that will be alleviated with
time.

The library also works very well with normal exception based code.
leaf::catch_ and leaf::exception_to_result
are useful abstractions that help the user integrate LEAF piece-by-piece
into the codebase which is important
when looking at updating existing code.

Because users can manually control the error context (including storing it
and using RAII-like wrappers to activate it),
it's usable in Asio which is a hard requirement for me.

Overall, the design of the library is quite powerful and is relatively
simple for how much it does.

>  - What is your evaluation of the implementation?

As a user, the only thing that matters to me is: is it better than I
could've done myself?

And the answer to that question is a resounding "yes!" Otherwise, the
implementation details
are best left evaluated by a fellow library author which I am not aiming to
be in this instance.

>  - What is your evaluation of the documentation?

Parts seemed out of line with what was in the examples (I'm thinking of
exception_to_result) but overall,
the documentation has good styling and goes into quite a bit of depth. The
examples in the source repo
are quite good as well. As a user, I found myself able to reference things
and get a working example
relatively quickly that show-cased the power of LEAF.

>  - What is your evaluation of the potential usefulness of the library?

Incredibly useful. When writing user-facing applications, it's important
that one gathers up appropriate data
to craft a useful error message to display. It's also incredibly useful
because of the type-safe matching in
try_handle_all.

Error codes are not sufficient many times and LEAF minimizes the amount of
boilerplate one would have
to use to update existing code.

>  - Did you try to use the library? With which compiler(s)? Did you
>    have any problems?

I did. It installed perfectly with CMake and was able to be
find_package()'d easily.

I used gcc 10.0.1 in WSL.

>  - How much effort did you put into your evaluation? A glance? A quick
>   reading? In-depth study?

I dedicated around 2 to 3 hours playing around with it in addition to
spending about an hour last
week reading the docs.

Even though I'm not fully adept with using the library, I see its
potential. Many Boost libs require
time to learn and digest and they're fantastic (I'm thinking of Asio,
Beast, Spirit) so I'm not holding
any stumbles I encountered against it.

>  - Are you knowledgeable about the problem domain?

If the problem is error handling, I'm decently well-versed. I've written
many user-facing applications
and have wound up reimplementing what LEAF does a couple of times.

Some closing thoughts...

LEAF doesn't have to be perfect to be accepted. I think as of now, it's
documented  and completed enough
that it can be picked up and used but more importantly, I think its core
goal is worthwhile enough that it should be
iterated on and refined by many users trying it out and providing their
feedback.

I've seen this pattern be re-implemented in Go and in many codebases I've
worked on professionally. LEAF far
and away has the best implementation I've personally seen and for that
reason, I think it should be wholeheartedly
accepted into Boost.

- Christian

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
123