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

7 Jun 2020

      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
     struct result_type_traits;

     template<typename T>
     struct result_type_traits<
       T,
       boost::mp11::mp_void<
        decltype(static_cast<bool>(std::declval<T>())),
        decltype(std::declval<T>().value()),
        decltype(std::declval<T>().error())
       >
     >
     {
       static bool has_error(T& x){return static_cast<bool>(x);}
       static decltype(std::declval<T>().value()) value(T& x){return 
x.value();}
       static decltype(std::declval<T>().error()) error(T& x){return 
x.error();}
     };

     template
     std::true_type  is_complete(T*);
     std::false_type is_complete(...);

     template<typename T>
     using 
is_result_type=decltype(is_complete(std::declval()));

So, if I have a result type that fits the requirements:

     struct my_result_type
     {
       operator bool(){return true;}
       int value(){return 0;}
       int error(){return 1;}
     };

then is_result_type::value is true automatically and
result_type_traits works out of the box. When I have 
something that
requires adaptation, specializing result_type_traits suffices:

     template<>
     struct result_type_traits
     {
       static bool has_error(FILE* p){return p==nullptr;}
       static FILE* value(FILE *p){return p;}
       static int error(FILE *p){return errno;}
     };

     // is_result_type::value is automatically true

In fact, with this approach is_result_type probably need not be a public 
type.
...
...
* 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){return 0;},
         [](boost::leaf::catch_std::exception){return 1;},
         [](const boost::leaf::error_info&){return 2;}
       ).value();
     }

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();
     }

(I've verified both compile ok).

Joaquín M López Muñoz