[review][LEAF] Review of LEAF starts today : May 22 - May 31
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
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 hodges.r@gmail.com office: +442032898513 home: +376841522 mobile: +376380212
On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost < boost@lists.boost.org> 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.
Thank you for your response. On Fri, 22 May 2020 at 21:30, Emil Dotchevski via Boost < boost@lists.boost.org> wrote:
On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost < boost@lists.boost.org> 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 hodges.r@gmail.com office: +442032898513 home: +376841522 mobile: +376380212
On Fri, May 29, 2020 at 1:10 AM Richard Hodges via Boost < boost@lists.boost.org> wrote:
Thank you for your response.
On Fri, 22 May 2020 at 21:30, Emil Dotchevski via Boost < boost@lists.boost.org> wrote:
On Fri, May 22, 2020 at 3:51 AM Richard Hodges via Boost < boost@lists.boost.org> 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?
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
Yes. I meant no dynamic allocation. The minimal bookkeeping is logically
similar to a tuple
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.
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.
On Sat, May 30, 2020 at 7:43 AM Bjorn Reese via Boost
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
On 31/05/2020 03:19, Emil Dotchevski wrote:
- At try_catch: a local tuple
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.
Speaking of removing duplicates... how are nested errors intended to be managed? I don't see any mention of this in the docs (but have possibly overlooked it). With exceptions we now have nested_exception, but they both might wish to transport a filename, for example, and these may be different at each level. At a particular catch site it may need to inspect or report both filenames (but they have to remain as separate entities associated with each error context, not merged into an e_relevant_file_names or similar).
On Mon, Jun 1, 2020 at 4:36 PM Gavin Lambert via Boost < boost@lists.boost.org> wrote:
With exceptions we now have nested_exception, but they both might wish to transport a filename, for example, and these may be different at each level.
Two options: 1) You want to capture the error objects reported by a lower level function, and then later look at them elsewhere. Here is an example on how to do this: With exceptions: https://github.com/zajo/leaf/blob/master/examples/capture_in_exception.cpp?t... . With leaf::result<T>: https://github.com/zajo/leaf/blob/master/examples/capture_in_result.cpp?ts=4 . This is analogous to nested exceptions, the exact context type is erased and the context is transported in an exception or in a leaf::result<T>. 2) A lower level function has failed but you don't want to report this as a new error so to speak, you just want to add more info to the existing failure. Ideally the two functions would use different error types, and in this case it'll just work, just use preload(). Otherwise, it might make sense to wrap the individual types that clash, rather than the whole context. Here is an example: try_catch( [&] { ...something that could throw... }, []( leaf::error_info const & e, leaf::e_file_name const & fn ) { e.error().load( e_wrapped_file_name { fn } ); throw; } ); This handler will execute for any exception that has e_file_name. We just wrap it in another type and rethrow. (The reason why this is needed is that a context can hold no more than one object for each type. In principle this can be changed, in which case you could have two e_file_name stored in the same context, associated with two different error_ids.)
On 2/06/2020 13:15, Emil Dotchevski wrote:
On Mon, Jun 1, 2020 at 4:36 PM Gavin Lambert wrote:
With exceptions we now have nested_exception, but they both might wish to transport a filename, for example, and these may be different at each level.
Two options:
1) You want to capture the error objects reported by a lower level function, and then later look at them elsewhere. Here is an example on how to do this:
With exceptions: https://github.com/zajo/leaf/blob/master/examples/capture_in_exception.cpp?t... . With leaf::result<T>: https://github.com/zajo/leaf/blob/master/examples/capture_in_result.cpp?ts=4 .
This is analogous to nested exceptions, the exact context type is erased and the context is transported in an exception or in a leaf::result<T>.
That appears to be parallel exceptions, not nested exceptions. I don't think these are analogous, given that remote_try_catch appears to be using some kind of magic to figure out which captured context to inspect, but you would instead presumably need to tell it explicitly whether you are looking at the context of the top-level exception or the nested exception.
2) A lower level function has failed but you don't want to report this as a new error so to speak, you just want to add more info to the existing failure. Ideally the two functions would use different error types, and in this case it'll just work, just use preload().
For a possible use case (I'm sure there are others), imagine a "safe file saver" class that uses backup files. The higher-level error reporting wants to report the filename that the caller was actually trying to save, and yet the underlying internal file API might want to report that the problem actually occurred when saving either the real file or the backup file. Obviously the low-level function doesn't know that it's dealing with a backup file, so it would just have a generic e_filename. And the higher-level function will likely want to report its intended filename as an e_filename as well, as that is the one the caller cares about. But does that mean that it would need to (and is this even possible?) move the underlying low-level filename to a new e_real_filename or similar in order to give e_filename a different value? This sort of thing seems like a flat error reporting model rather than a nested model, and flat errors tend to lose information. (Not that C++'s mechanisms for nested exceptions are particularly wonderful.)
(The reason why this is needed is that a context can hold no more than one object for each type. In principle this can be changed, in which case you could have two e_file_name stored in the same context, associated with two different error_ids.)
Presumably this would not be possible without either reintroducing memory allocation or pre-declaring the max number of possible nested errors either per-context or globally.
On Mon, Jun 1, 2020 at 6:54 PM Gavin Lambert via Boost < boost@lists.boost.org> wrote:
This is analogous to nested exceptions, the exact context type is erased and the context is transported in an exception or in a leaf::result<T>.
That appears to be parallel exceptions, not nested exceptions. I don't think these are analogous, given that remote_try_catch appears to be using some kind of magic to figure out which captured context to inspect, but you would instead presumably need to tell it explicitly whether you are looking at the context of the top-level exception or the nested exception.
I see why you say it's not like nested exception. What you refer as magic
in remote_try_catch is actually simple: it itself has a local context<>,
and the internal machinery detects that the passed result<T> (or the
exception) wraps a context in it, and tells the wrapped context, through a
virtual function call, to spill its content which is captured in the local
context, and from then on everything proceeds as usual.
I'll add that the the lowest level API exposed by LEAF works directly with
the context<> type. You could:
leaf::context
2) A lower level function has failed but you don't want to report this as a new error so to speak, you just want to add more info to the existing failure. Ideally the two functions would use different error types, and in this case it'll just work, just use preload().
For a possible use case (I'm sure there are others), imagine a "safe file saver" class that uses backup files. The higher-level error reporting wants to report the filename that the caller was actually trying to save, and yet the underlying internal file API might want to report that the problem actually occurred when saving either the real file or the backup file.
Obviously the low-level function doesn't know that it's dealing with a backup file, so it would just have a generic e_filename.
And the higher-level function will likely want to report its intended filename as an e_filename as well, as that is the one the caller cares about.
But does that mean that it would need to (and is this even possible?) move the underlying low-level filename to a new e_real_filename or similar in order to give e_filename a different value?
Yes, possible. You write a handler that takes leaf::error_info, call its .error() to obtain the error_id of this error, then .load the new value for e_file_name.
This sort of thing seems like a flat error reporting model rather than a nested model, and flat errors tend to lose information. (Not that C++'s mechanisms for nested exceptions are particularly wonderful.)
Indeed. I try to use different types for different things. You're saying that both libs would use leaf::e_file_name, but really they shouldn't, it'd be better if each defines a type within its own namespace. After all, the reason why "flat" error reporting tends to lose information is that there isn't efficient storage to send everything over. With LEAF, there is -- just make sure all error info is properly namespaced.
(The reason why this is needed is that a context can hold no more than one object for each type. In principle this can be changed, in which case you could have two e_file_name stored in the same context, associated with two different error_ids.)
Presumably this would not be possible without either reintroducing memory allocation or pre-declaring the max number of possible nested errors either per-context or globally.
Yes, per context. But really I don't want to do that, it's rather inelegant.
On 2020-06-02 04:37, Emil Dotchevski via Boost wrote:
I'll add that the the lowest level API exposed by LEAF works directly with the context<> type. You could:
leaf::context
ctx; ctx.activate(); .....; // call functions that report E1, E2, E3 errors, obtain an error_id ctx.deactivate(); .....; // possibly move ctx somewhere else R r = ctx.handle_error<R>(id, <list of error handlers>); I'm showing this because depending on the use case it might be useful; context<> is very similar to tuple<>, and so you could move it about and handle errors later. As I type this, i realize that Bjorn is right that this needs a get<I> interface.
I somehow missed the context::handle_error() function during the review. This addresses my main concern about avoiding wrapping the "TryBlock". Review manager: This changes my conditional accept into an unconditional accept.
On 6/3/20 07:23, Bjorn Reese via Boost wrote:
On 2020-06-02 04:37, Emil Dotchevski via Boost wrote:
I'll add that the the lowest level API exposed by LEAF works directly with the context<> type. You could:
leaf::context
ctx; ctx.activate(); .....; // call functions that report E1, E2, E3 errors, obtain an error_id ctx.deactivate(); .....; // possibly move ctx somewhere else R r = ctx.handle_error<R>(id, <list of error handlers>); I'm showing this because depending on the use case it might be useful; context<> is very similar to tuple<>, and so you could move it about and handle errors later. As I type this, i realize that Bjorn is right that this needs a get<I> interface.
I somehow missed the context::handle_error() function during the review. This addresses my main concern about avoiding wrapping the "TryBlock".
Review manager: This changes my conditional accept into an unconditional accept.
Thank you for the clarification Bjorn. -- Michael Caisse Ciere Consulting ciere.com
- 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
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. Doesn't this state the opposite of "generally, no" or am I missing anything? If I understand this correctly, in all cases (so also in the happy case) a tuple is initialized. The slot<> class sounds like an optional, so the "int"s to be zeroed are not contiguous, similar with the TLS pointer. Not sure how trivial initializing the latter is but this sounds like (stack) memory and runtime overhead. So I think the request for a comparison of the happy cases for LEAF and exceptions is justified.
On Tue, Jun 2, 2020 at 2:32 AM Alexander Grund via Boost < boost@lists.boost.org> wrote:
By "generally no" I meant that the work is mostly limited to try_catch. <deleted details> Doesn't this state the opposite of "generally, no" or am I missing anything?
The happy path includes everything that happens when there is no exception thrown, not only what happens at the try...catch, that's what I meant.
So I think the request for a comparison of the happy cases for LEAF and exceptions is justified.
A benchmark can be added, sure. Here's the initialization code generated by
leaf::try_catch that deals with a context that has a struct e_info { int
value; }:
mov dword ptr [rsp + 8], 0
mov rax, qword ptr fs:[0]
lea rax, [rax + boost::leaf::leaf_detail::slot
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 (rainerd@eldwood.com)
On Sun, May 24, 2020 at 1:14 AM Rainer Deyke via Boost < boost@lists.boost.org> 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).
Disclaimer: This is from someone not using Boost.Exception, Expected or any other resultlibrary 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 inhibits (N)RVO (even more for C++17 and up). This is my main critic point about result 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.
On Fri, May 29, 2020 at 1:26 AM Alexander Grund via Boost < boost@lists.boost.org> 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!
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. Naive approach: Make the aggregate handler a type with variadic template
params for the handlers and overload try_handle_all based on that. One can then call the other to avoid code duplication I'd also like to remind you of the first part of the proposal where `remote_handle_all` does not require the user to create a lambda which IMO considerably improves the UX. Reason for bringing this up again is that once it is released it can't really be changed (or only at a higher cost) and IMO eliminating something already identified as "unintuitive" is worth delaying the release for a bit for exploring other approaches.
- "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 { } Ok, so you are referring to diamond-style inheritance hierarchies which are for the reason you mentioned often avoided. I feel like this doesn't come up to often for exception types as they are usually flat hierarchies (speaking from my experience, YMMV) Hence I'd recommend not bringing this up in the documentation as I find it more distracting than helpful.
- "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). Thank you for the explanation. Can you show an example in the docu for a use case? I'm having a hard time imagining one as usually errors are more like "I failed to open file x" and not "I failed to open file x, y & z" Having a more complete example use case where accumulate is actually used to store (and retrieve) multiple values could be helpful for the user - "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. May I suggest to make this more clear in the documentation? Like "On error, the function is called in the destructor, else it is not called" int that sentence. 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>.
Sorry I might have been not fully clear, allow me to be more specific: I wasn't referring to lead::result<T> but to the concept of result<T> as found in this and other exception handling libraries. With an API like `result<Foo> do() noexcept` instead of `Foo do()` you won't get (N)RVO when you return a `Foo` (in the happy case) as the standard only allows it when the types are exactly the same. So my question was what impact the missing RVO has on performance in the happy path compared to the "standard" approach of using RVO-enabled functions and (potential) exceptions.
Alexander Grund wrote:
With an API like `result<Foo> do() noexcept` instead of `Foo do()` you won't get (N)RVO when you return a `Foo` (in the happy case) as the standard only allows it when the types are exactly the same.
This doesn't matter much nowadays. E.g. in https://godbolt.org/z/zC-Cah if you remove x.f(), x goes away.
Am 02.06.20 um 15:38 schrieb Peter Dimov via Boost:
Alexander Grund wrote:
With an API like `result<Foo> do() noexcept` instead of `Foo do()` you won't get (N)RVO when you return a `Foo` (in the happy case) as the standard only allows it when the types are exactly the same.
This doesn't matter much nowadays. E.g. in https://godbolt.org/z/zC-Cah if you remove x.f(), x goes away. Not sure if that really proves the point. I think you need to compare a result<X> function to a function returning X directly. Like so: https://godbolt.org/z/JFmn_j
And for that there is a difference: There are 4 main memory accesses in the result<X> function and none in the "regular" one. Writing and reading those values likely influences performance for the worse. However it looks like for large objects (like Y above) there seems to be no difference indeed and it very much looks like RVO is happening. Didn't expect that. So there is only a difference for small and trivial types which will be passed through RAM instead of registers.
Alexander Grund wrote:
Not sure if that really proves the point. I think you need to compare a result<X> function to a function returning X directly. Like so: https://godbolt.org/z/JFmn_j
And for that there is a difference: There are 4 main memory accesses in the result<X> function and none in the "regular" one.
That's not because of NRVO, it's because X fits in registers and result<X> doesn't.
- 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
On Fri, May 29, 2020 at 6:20 AM Niall Douglas via Boost < boost@lists.boost.org> 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. :)
- 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.
On Fri, May 29, 2020 at 10:27 PM Krystian Stasiowski via Boost < boost@lists.boost.org> 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_
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_.
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
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
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).
*Review:* *Introduction:* This is my first review of a boost candidate library. Anyone who has done the work in order to write, document and present a library for inclusion is already in my view far more authoritative than me, a mere maintainer. This review pivots on the question of, "what is the purpose of Boost?" a) A collection of high quality libraries that meet the needs of demonstrated use cases, or b) A high quality sandbox for ideas that may eventually influence the direction of the language or the standard library. In the past, I think it has been both. Which has been a source of both criticism and praise across the user spectrum. Boost seems to me the "Marmite*" of the C++ user community - people either hate it on grounds of "bloat" or love it on grounds of quality, usefulness and being a source of inspiration. All Boost libraries share a common trait, which is that the authors are feidishly clever people, with insights beyond those of an above-average developer, Many Boost libraries brilliantly answer (or answered in the past) demonstrable use cases. Some examples of these (off the top of my head) are: Asio, Beast, Spirit, Container, Mpl, Mp11, Smart Pointer, Bimap, Multi Index, Regex, and many others. Some Boost libraries are brilliantly clever and I often wish that either I had a use case for them, or could be bothered to do the work needed order to use them: Hana, Units, Type Erasure and so on. Each library we add to Boost represents maintenance overhead for the people who give their time in order to ensure that releases are seamless and professional. So in a sense, each Boost library added ought to at least offer some credible payoff for a significant subset of Boost users, future or present. *The task at hand:*
- What is your evaluation of the design?
* LEAF represents a comprehensive error handling strategy which seeks to encapsulate the strengths of all others.
- What is your evaluation of the implementation?
* I personally find the lambda-based pattern-matching handler code to be utterly impenetrable. I accept that it is solving a problem that is beyond the scope of the language in which it is written, and this has a cost in terms of readability and accessibility to a (LEAF) novice.
- What is your evaluation of the documentation?
* LEAF is well documented and is maintained and presented by a motivated and responsive author.
- What is your evaluation of the potential usefulness of the library?
* I have not seen a credible use case articulated or demonstrated to me. This in my mind, puts leaf in category b, "A high quality sandbox for ideas that may eventually influence the direction of the language or the standard library."
- Did you try to use the library? With which compiler(s)? Did you have any problems?
Godbolt, gcc. No problems.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
In depth study, questions to the author around some implementation details and use cases.
- Are you knowledgeable about the problem domain?
Yes And so to the final question. ACCEPT or REJECT? It's a difficult call, and the answer depends on what is our consensus on the purpose of Boost. a) A collection of high quality libraries that meet the needs of demonstrated use cases, or In this case, brilliant as it is, I have not yet seen an example of a motivating use case sufficuently compelling to say ACCEPT. In which case the added workload of a new, cryptic library that represents a paradigm shift in the way people think about errors and is therefore unlikely to be used very much indicates an answer of REJECT. With an important footnote to the effect that once there is some demonstrated *de facto* user traction or compelling presentation of use case where LEAF clearly outperforms existing methods, I would switch to ACCEPT. b) A high quality sandbox for ideas that may eventually influence the direction of the language or the standard library. In this case, LEAF would demonstrate value at the present time, and my recommendaton, on grounds of quality, would be ACCEPT, even though I'll probably never use it. *Summary:* My apologies to my colleagues, I am in need of some direction before I can make my final recommendation. Are we today in camp A or camp B? In the light of your guidance, my answer is: if A, REJECT (happy to revisit in the light of *de facto* traction or convicing use case) if B, ACCEPT Regards, R * Marmite is the brand name of a black yeast derivative that is sold in jars in the UK. The country is divided as to whether it is revolting or delicious. The manufacturer's TV advertising efforts famously capitalised on this conflict.
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 hodges.r@gmail.com office: +442032898513 home: +376841522 mobile: +376380212
It appears that the initial impression of the error handling syntax
is off-puting for some reviewers, which is understandable. However, some
see a semantic problem with the lambda functions. So let me clarify.
A Boost Exception-style interface would work like this, for some container
of error info e:
catch( .... )
{
if( my_info const * info = get_error_info
El 22/05/2020 a las 11:31, Michael Caisse via Boost escribió:
The Boost formal review of Emil Dotchevski's LEAF (Lightweight Error Augmentation Framework) library will take place from May 22 through May 31st.
Hi, this is my late, unexhaustive review of LEAF. I didn't have the time
to make
this more structured, so each section below is more of a list of issues
and ideas that
I came up with while reading the docs and inspecting the code.
*Design*
* If I understood it right, leaf::context, leaf::new_error and handlers
can only take
E-type arguments. Why is this so? I'm sure there's a reason for that,
but at first blush
seems like anything (which is moveable etc.) could be potentially
accepted and
automatically handled by the framework, which would eliminate the need
for some
user-side boilerplate.
* I feel like is_result_type should be converted into a traits class
(like allocator_traits)
with value() and error() static functions so as to make it easier to
adapt external
error types into the framework. Also, instead of is_result_type<T>
specializing to
std::false_type by default, some metaprogramming gymnastics can be
played to try
to deduce if T provides a compatible interface.
* Using LEAF requires that called functions be wrapped/refactored so as
to return a
leaf::result<T> value. Given that users are already expected to use
LEAF_AUTO
to reduce boilerplate, maybe this macro can be augmented like this:
namespace leaf{
template <typename T>
[[nodiscard]] result<T> wrap_result(result<T>&& x){return std::move(x);}
template <typename T>
[[nodiscard]] result<T> wrap_result(T&& x){return ...;} // figure out
what values of T are an error
#define LEAF_AUTO(v,r)\
auto && _r_##v = leaf::wrap_result(r);\
if( !_r_##v )\
return _r_##v.error();\
auto & v = _r_##v.value()
}
where non leaf::result-returning functions are automatically wrapped.
The tricky
part is the second overload of wrap_result, which could have some
default semantics
for what constitutes an erroneous value (for instance, !x evaluating to
true) and then
be overloadable by the user: defining the semantics for
wrap_value
On Sat, Jun 6, 2020 at 4:21 AM Joaquin M López Muñoz via Boost < boost@lists.boost.org> wrote:
* If I understood it right, leaf::context, leaf::new_error and handlers can only take E-type arguments. Why is this so?
No longer the case; is_e_type is deleted on develop (the review branch remains unchanged to avoid confusion).
* I feel like is_result_type should be converted into a traits class (like allocator_traits) with value() and error() static functions so as to make it easier to adapt external error types into the framework.
Possibly. I prefer to not add complexity that is not strictly needed.
* Using LEAF requires that called functions be wrapped/refactored so as to return a leaf::result<T> value.
Not a requirement. In fact one of the main design goals of LEAF is to support transporting error objects of arbitrary types through intermediate uncooperative layers of functions.
Given that users are already expected to use LEAF_AUTO to reduce boilerplate, maybe this macro can be augmented like this:
namespace leaf{ template <typename T> [[nodiscard]] result<T> wrap_result(result<T>&& x){return std::move(x);} template <typename T> [[nodiscard]] result<T> wrap_result(T&& x){return ...;} // figure out what values of T are an error
Possible, however I'm exploring what I think is a better option to enable this functionality. It is possible to make LEAF able use non-result<T> types directly, without having to wrap them.
* Maybe LEAF_AUTO and LEAF_CHECK can be unified into a single, variadic macro. Just a naming observation here.
* Maybe there should be a (BOOST_)LEAF_ASSIGN macro to cover this:
LEAF_AUTO(f, file_open(file_name)); ... // closes f LEAF_ASSIGN(f,file_open(another_file_name));
Possible, but I'd rather not facilitate reusing of local variables.
* Whis is there a need for having separate try_handle_all and try_handle_some? Isn't try_handle_some basically equivalent to try_handle_all with a [](leaf::error_info const & unmatched) handler?
Two reasons: 1) try_handle_all ensures at compile time that the user has provided a "catch all" handler. Consider that without this requirement, under maintenance someone may delete it and now the program has a subtle, difficult to detect bug. 2) because try_handle_all knows all errors are handled, it can unwrap the result type and return a value.
* I may be wrong, but I feel like error-handling and exception are completely separate. Is it not possible to have try_handle_all handle both [](leaf::match<...>) and [](leaf::catch_<...>) handers? The framework could statically determine whether there's a catch handler and then insert its try{try_block()}catch{...} thing only in this case, so as to be exception-agnostic when needed.
That is exactly how LEAF works. Use catch_ in any of your handlers, and try_handle_all/some will catch exceptions, otherwise they won't (the third alternative, try_catch, always catches exceptions, and does not use a result type).
I'm focusing here on code/naming guidelines in the context of Boost libraries:
* LEAF macros (both public and internal) should be BOOST_-prefixed.
Of course.
* There are macros with the same semantics as Boost-level macros: BOOST_NO_EXCEPTIONS, BOOST_NO_THREADS. The latter should be used insstead i this is to become a Boost library.
Yes, integration with Boost Config is TBD.
* There are chunks of Boost source code embedded into the project (test/boost/core). Needless to say this should be removed.
Fixed on develop.
* s/lEAF_NODISCARD/LEAF_NODISCARD in
https://github.com/zajo/leaf/blob/76edf8edc3a17be3ac4064f3cc18d7c6a4e86b4c/i... Ouch. Thanks, fixed on develop.
* LEAF_AUTO(v,r) defines a variable with name _r_##v. Identifiers beginning with "_r" are not strictly reserved by the C++ standard (they are reserved only at global scope and within std namespace), but I think it is bad practice to have them anyway.
The idea is to minimize the chance of a name clash, hoping that users know to avoid such names
etc. Although the docs do not stress it, I think it can also play a role when mixing error-returning and exception-throwing code.
The docs mention this. There is also leaf::exception_to_result, see this example: https://github.com/zajo/leaf/blob/master/examples/exception_to_result.cpp?ts... Thanks!
El 06/06/2020 a las 22:53, Emil Dotchevski via Boost escribió:
On Sat, Jun 6, 2020 at 4:21 AM Joaquin M López Muñoz via Boost < boost@lists.boost.org> wrote:
[...] * I feel like is_result_type should be converted into a traits class (like allocator_traits) with value() and error() static functions so as to make it easier to adapt external error types into the framework. Possibly. I prefer to not add complexity that is not strictly needed.
I'd say this doesn't add complexity for the user, but on the contrary
makes it
easier to detect and adapt result types. I'm thinking about something
like this:
template
* Using LEAF requires that called functions be wrapped/refactored so as to return a leaf::result<T> value. Not a requirement. In fact one of the main design goals of LEAF is to support transporting error objects of arbitrary types through intermediate uncooperative layers of functions.
Yes, right.
Given that users are already expected to use LEAF_AUTO to reduce boilerplate, maybe this macro can be augmented like this:
namespace leaf{ template <typename T> [[nodiscard]] result<T> wrap_result(result<T>&& x){return std::move(x);} template <typename T> [[nodiscard]] result<T> wrap_result(T&& x){return ...;} // figure out what values of T are an error Possible, however I'm exploring what I think is a better option to enable this functionality. It is possible to make LEAF able use non-result<T> types directly, without having to wrap them.
I'd say result_type_traits would enable this, in fact.
* Maybe LEAF_AUTO and LEAF_CHECK can be unified into a single, variadic macro. Just a naming observation here.
* Maybe there should be a (BOOST_)LEAF_ASSIGN macro to cover this:
LEAF_AUTO(f, file_open(file_name)); ... // closes f LEAF_ASSIGN(f,file_open(another_file_name)); Possible, but I'd rather not facilitate reusing of local variables.
Not sure I'm explaining myself... Perfectly valid code (the majority, I'd say) assigns values to local variables after the variable has been constructed. LEAF_AUTO hides away error handling on construction, but there is no equivalent for assignment. So the LEAF user would have to write workarounds like: LEAF_AUTO(f, file_open(file_name)); ... // closes f LEAF_AUTO(f2,file_open(another_file_name)); f=f2; or worse yet handle error checking explicitly.
* Whis is there a need for having separate try_handle_all and try_handle_some? Isn't try_handle_some basically equivalent to try_handle_all with a [](leaf::error_info const & unmatched) handler? Two reasons:
1) try_handle_all ensures at compile time that the user has provided a "catch all" handler. Consider that without this requirement, under maintenance someone may delete it and now the program has a subtle, difficult to detect bug.
2) because try_handle_all knows all errors are handled, it can unwrap the result type and return a value.
I see, thank you. Didn't notice try_handle_all and try_handle_some return types are different, not sure this is a beautiful decision designwise but it's ok, but this is more of a personal opinion I guess.
* I may be wrong, but I feel like error-handling and exception are completely separate. Is it not possible to have try_handle_all handle both [](leaf::match<...>) and [](leaf::catch_<...>) handers? The framework could statically determine whether there's a catch handler and then insert its try{try_block()}catch{...} thing only in this case, so as to be exception-agnostic when needed. That is exactly how LEAF works. Use catch_ in any of your handlers, and try_handle_all/some will catch exceptions, otherwise they won't (the third alternative, try_catch, always catches exceptions, and does not use a result type).
I still don't get it. What is the difference between:
int main()
{
return boost::leaf::try_handle_some(
[]()->boost::leaf::result<int>{
return 0;
},
[](boost::leaf::match
El 07/06/2020 a las 11:00, Joaquin M López Muñoz escribió:
I'd say this doesn't add complexity for the user, but on the contrary makes it easier to detect and adapt result types. I'm thinking about something like this:
[...] static bool has_error(T& x){return static_cast<bool>(x);}
This should be static bool has_error(T& x){return !static_cast<bool>(x);} Of course. Another question: why does leaf::catch_<E> require that E be derived from std::exception? Can't I throw any arbitrary type I please? Joaquín M López Muñoz
On Sun, Jun 7, 2020 at 2:02 AM Joaquin M López Muñoz < joaquinlopezmunoz@gmail.com> wrote:
I still don't get it. What is the difference between:
int main() { return boost::leaf::try_handle_some( []()->boost::leaf::result<int>{ return 0; }, [](boost::leaf::match
){return 0;}, [](boost::leaf::catch_std::exception){return 1;}, [](const boost::leaf::error_info&){return 2;} ).value(); }
One of your handlers uses catch_<>. This means that your try block will execute inside a try scope, and your handlers will execute in a catch scope. If the try block throws, LEAF will catch the exception and attempt to find a handler. It doesn't have to be a handler that uses catch_<>, the first suitable handler will be called. If your try block returns a failure without throwing, LEAF will attempt to find a handler also. In this case, the handler that uses catch_<> will not be called, because no exception was thrown. If a suitable handler could not be found, the error is propagated by rethrowing the exception or returning the result<int> returned from the try block, as appropriate.
and
int main() { return boost::leaf::try_catch( []()->boost::leaf::result<int>{ return 0; }, [](boost::leaf::match
){return 0;}, [](boost::leaf::catch_std::exception){return 1;}, [](const boost::leaf::error_info&){return 2;} ).value(); }
With try_catch, your try block always executes in a try scope, and your handlers execute in a catch scope, even if none of them use catch_<>. If an exception is thrown, try_catch will try to find a suitable handler. It does not understand the semantics of the result type.
El 07/06/2020 a las 20:00, Emil Dotchevski escribió:
On Sun, Jun 7, 2020 at 2:02 AM Joaquin M López Muñoz
wrote: I still don't get it. What is the difference between:
int main() { return boost::leaf::try_handle_some( []()->boost::leaf::result<int>{ return 0; }, [](boost::leaf::match
){return 0;}, [](boost::leaf::catch_std::exception){return 1;}, [](const boost::leaf::error_info&){return 2;} ).value(); } One of your handlers uses catch_<>. This means that your try block will execute inside a try scope, and your handlers will execute in a catch scope.
If the try block throws, LEAF will catch the exception and attempt to find a handler. It doesn't have to be a handler that uses catch_<>, the first suitable handler will be called.
How can a non-catch_ handler be suitable when an exception has been thrown?
If your try block returns a failure without throwing, LEAF will attempt to find a handler also. In this case, the handler that uses catch_<> will not be called, because no exception was thrown.
Understood.
If a suitable handler could not be found, the error is propagated by rethrowing the exception or returning the result<int> returned from the try block, as appropriate.
Understood.
and
int main() { return boost::leaf::try_catch( []()->boost::leaf::result<int>{ return 0; }, [](boost::leaf::match
){return 0;}, [](boost::leaf::catch_std::exception){return 1;}, [](const boost::leaf::error_info&){return 2;} ).value(); } With try_catch, your try block always executes in a try scope, and your handlers execute in a catch scope, even if none of them use catch_<>. If an exception is thrown, try_catch will try to find a suitable handler. It does not understand the semantics of the result type.
I see that, indeed, try_catch does not use non-catch_ handlers:
auto a=try_handle_some(
[]()->result<int>{
return new_error(0);
},
[](match
If the try block throws, LEAF will catch the exception and attempt to find a handler. It doesn't have to be a handler that uses catch_<>, the first suitable handler will be called.
How can a non-catch_ handler be suitable when an exception has been
On Sun, Jun 7, 2020 at 12:19 PM Joaquín M López Muñoz < joaquinlopezmunoz@gmail.com> wrote: thrown? This fits the design. Handlers are considered in order (like catch statements are in C++), and the first one LEAF can supply with arguments (based on available info) is called to handle the error. If a handler doesn't take a catch_<> argument, it means it doesn't need the exception object in order to handle the error.
So, what's the point of allowing non-catch_ handlers in try_catch?
Also, seems like try_handle_some provides a superset of the functionality offered by
The exact type of the exception object may be irrelevant, because you have the option to use use all the other object(s) associated with the failure to discriminate between different errors. try_catch, right? In which
situations would a user need try_catch because try_handle_some does not fit the bill?
try_handle_some/all require your try block to return a result<T> of some sort, and using catch_<> with them is just a way to write unified error handling in cases when exceptions are also possible. try_catch has no such requirement.
El 07/06/2020 a las 22:19, Emil Dotchevski via Boost escribió:
On Sun, Jun 7, 2020 at 12:19 PM Joaquín M López Muñoz < joaquinlopezmunoz@gmail.com> wrote:
Emil Dotchevski via Boost escribió
If the try block throws, LEAF will catch the exception and attempt to find a handler. It doesn't have to be a handler that uses catch_<>, the first suitable handler will be called. How can a non-catch_ handler be suitable when an exception has been thrown?
This fits the design. Handlers are considered in order (like catch statements are in C++), and the first one LEAF can supply with arguments (based on available info) is called to handle the error. If a handler doesn't take a catch_<> argument, it means it doesn't need the exception object in order to handle the error.
Still don't get it. Can you provide an example where throwing an exception within try_handle_* results in resolution by a non-catch_ handler?
So, what's the point of allowing non-catch_ handlers in try_catch? The exact type of the exception object may be irrelevant, because you have the option to use use all the other object(s) associated with the failure to discriminate between different errors.
Can you provide an example where a non-catch_ handler is effectively used in a try_catch statement?
Also, seems like try_handle_some provides a superset of the functionality offered by try_catch, right? In which situations would a user need try_catch because try_handle_some does not fit the bill?
try_handle_some/all require your try block to return a result<T> of some sort, and using catch_<> with them is just a way to write unified error handling in cases when exceptions are also possible.
try_catch has no such requirement.
So, if I'm getting this right, the benefit of try_catch vs. try_handle_* is that the returned type need not be a result<T> (or similar) thing. Is this right? Can you explain the benefits of using try_catch vs. resorting to language-level "real" try and catch blocks? Joaquín M López Muñoz
On Sun, Jun 7, 2020 at 2:19 PM Joaquin M López Muñoz < joaquinlopezmunoz@gmail.com> wrote:
So, if I'm getting this right, the benefit of try_catch vs. try_handle_* is that the returned type need not be a result<T> (or similar) thing. Is this right?
Can you explain the benefits of using try_catch vs. resorting to language-level "real" try and catch blocks?
One benefit is that you can associate arbitrary error objects with
exceptions, and dispatch error handling based on their availability. For
example, if you use Boost Exception, you could say:
try
{
f();
}
catch( read_error const & e )
{
if( auto const * fn = boost::get_error_info
participants (14)
-
Alexander Grund
-
Bjorn Reese
-
Christian Mazakas
-
Emil Dotchevski
-
Gavin Lambert
-
Joaquin M López Muñoz
-
Joaquín M López Muñoz
-
Krystian Stasiowski
-
Michael Caisse
-
Niall Douglas
-
Peter Dimov
-
Rainer Deyke
-
Richard Hodges
-
Vinnie Falco