Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)

Subject: Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)
From: Emil Dotchevski (emildotchevski_at_[hidden])
Date: 2018-02-02 03:58:03

On Thu, Feb 1, 2018 at 5:29 PM, Gavin Lambert via Boost <
boost_at_[hidden]> wrote:

> On 1/02/2018 19:55, Emil Dotchevski wrote:
>
>> This does not protect the user at all:
>>
>> std::unique_ptr<A> a=A::create();
>> a->foo(); //undefined behavior
>>
>
> Sure, but you can always write daft code.  If you are aware that it is
> legal for A::create() to return nullptr (and you should be aware, because
> it will be documented), then the above would be an obvious bug, and you
> should replace it with:
>
>     if (auto a = A::create())
>     {
>         a->foo();
>     }
>
> This is *much* more obviously safe than your suggested exception-enabled
> code, where you have to have external knowledge (that it will throw on
> failure and cannot ever return nullptr).  And it's trivial to add logic for
> the failure case in an else block, vs. the more wordy try-catch.
>

You don't use try-catch often at all in C++, very few places need it. In
the case you need to handle the failure locally, it is not all that verbose:

try
{
  auto a=A::create();
  a->foo();
}
catch(...)
{
}

vs.

if( auto a=A::create() )
{
  a->foo();
}
else
{
}

Obviously you have to know that A::create() throws on error, however it is
not inherently clearer to use the explicit if, because you don't know if
you're checking for failure or checking for null ptr that isn't a failure.
Yes, this is a bit silly in case of a function that is called create, hence
the if is silly too if you know that it'll throw on error.

Granted, exceptions are better at telling you *why* it failed, which the
> above can't.  And exceptions are better at hiding the error-handling paths
> for the cases where errors are expected to be exceptionally rare (pun
> intended)

It has nothing to do with how rare it is. If there is a useful
postcondition to be enforced, you throw, even if it happens frequently.

Nobody is forcing you to stop using exceptions, or even encouraging the
> majority of applications to stop using them.  Outcome just provides a way
> to add back a little (or a lot, as needed) explanation to failure in cases
> where someone has already decided they can't or don't want to use
> exceptions for whatever reason

I've expressed my objections already, but see below.

> This is not an advantage over using exceptions, and besides it is only true
>> in the simplest of cases. The problem is that Outcome requires users to
>> fit
>> any and all possible failures into a single error type, which is not
>> always
>> possible. In general, you can neither enumerate nor reason about all
>> failures which may occur. Notably, the one Outcome feature that can deal
>> with this problem is its ability to transport std::exception_ptr. :)
>>
>
> One error type per context.  You can have completely different error
> enumerations per method if you really want to, although it would probably
> be more common to have one error domain per library, or just use the
> closest approximation from a standard set, like POSIX errno.  Or use
> error_code, which allows relatively seamless interoperation with all of
> these.
>

You're missing my point. I am not saying that one can't design an error
type to use with result<T,E>, which could accomodate all of the cases he
cares about, but that if you have a low level library that gives you
result<T,E1> and you need to return result<T,E2>, you'll be remembering why
exception specifications were and still are a horrible idea. Consider that
in a generic context you may not even know exactly what E1 is -- it could
be a template parameter.

Ideally, the erorr type should be erased, that is, it should not be a
parameter of the result template, but of the result constructor, so that
error-neutral contexts can just forward _any_ error to the caller. This is
not easy to do. I tried to do this, but the result<T> type I ended up with
wasn't efficient enough to return at every function call. This lead me to
the realization that it's a bit silly to return the error object one level
at a time, because usually the caller only needs it to return it to its
caller. Ultimately, this lead me to using TLS, and the result is (Boost)
Noexcept.

Alternatively, removing the flexibility to use different error types and
sticking to error_code *only* may be good enough; after all, its design
goal is to be able to represent all possible error conditions. My instincts
are that this is not ideal, I've always thought that error_code is not an
error object, it's a thing to put into an error object. But I might be
wrong, I don't know.

Regardless, using a template parameter is a bad idea IMO -- and adding
policies is even worse.

> In most code, the best you can do with them is catch them in some central
> place and log them for some developer to look at later.

That is not true at all. You catch exceptions when you can recover from
them. Semantically, it's as if after every function call the compiler
automatically inserts:

if( error ) return error;

...except in the case when you can handle the error, in which case you
write a different if statement, in the form of try..catch. Nothing more,
nothing less.

>   Too many methods just throw one of the standard types like
> std::runtime_error without defining custom subclasses or adding anything
> more programmatically useful beyond the text message

This is plain stupid. Equally stupid would be to return result<T,char const
*>. Just don't do it.

Emil