Re: [boost] [outcome] High level summary of review feedback accepted so far

Subject: Re: [boost] [outcome] High level summary of review feedback accepted so far
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-05-28 22:39:58

> Niall I'm trying to see what you want to propose by difference to a
> hypothetical generic interface to see what is the added value.

Sure. We are trying to achieve different things though. Your Expected
needs to be a superb primitive building block useful for making other
stuff. It's why I would be much happier with narrow contracts on your
observers, it's easy for someone to derive from expected<T, E> locally
and override narrow observers with wide observers, or add new member
functions with wide contracts.

The harder question for you is whether to supply that wide observer
alternative yourself. Do you supply a completely different name of type
to help public APIs distinguish which contract is being supplied? Or do
you provide a second, wide set of observer member functions?

These are hard choices.

>> .has_error() is true if and only if the current state is an
>> error_code_extended instance.
> Okay. Why don't you name it has_error_code?
> For me having an exception_ptr stored was also an error.

My thinking was that an exception isn't an error. It's *exceptional*. An
error is just well, an error. It is not as strong.

>>> Maybe, outcome should have a get_state function that returns an enum so
>>> that the user can do a switch.
>> I had previously a .visit(callable) for that.
> Any sum type should provide a visit function. I've implemented it in the
> std_make repository. I will add it to Expected proposal once we have a
> SumType type of classes.

My sole reason for leaving it out of the presented library (even though
it's actually in the source code, just disabled) was compile time
impact. You can easily make do without it.

>>>> (NOTE: expected<T, E> will track whatever LEWG Expected does, but it
>>>> differs from what will become result<T> by having a wide contract on
>>>> .value() and narrow contracts on operator*(), .error(), operator->().
>>>> result<T> will have narrow contracts on everything, it is basically a
>>>> thin wrap of std::variant<T, error_code_extended> except with strong
>>>> never empty warranty)
>>> What will be the differences between result<T> and expected<T,
>>> error_code_extended>?
>>>
>>> The wide contracts for the observers? Can not we provide wide and narrow
>>> contracts or don't reuse the same name with different meaning?
>> The description above quite literally tells you the differences.
>
> Why we cannot provide wide and narrow observers? If this is the single
> difference I don't see the need for having two types.

The whole point behind Outcome's wide observers with default actions is
to save typing boilerplate. Typing more code to get more safety seems
the wrong way round to me. It should always be the incentive that to get
*less* safety the programmer must explicitly type more code.

Now if you agree with that, would the following change to Expected be
acceptable:

- value_type& .value() - throws bad_expected_access<error_type> if not
valued

- error_type .error() - returns default constructed error_type if not
errored

- value_type& .value_raw() - reinterpret_cast<value_type&>

- error_type& .error_raw() - reinterpret_cast<error_type&>

If you feel you can reach this, or anything where less safety forces the
programmer to type more, I can match that in Outcome.

>> I would assume an expected<T, E1, .., En> could only return a
>> std::variant<E1, ..., En> from its .error(). I can't think what else it
>> could do.
> This function *could* return  variant<E1, ..., En> or variant<E1&, ...,
> En&> or any sum type that represents the error, that is the Not-A-Value.
> 
> Maybe, expected<T, E1, ...>::get_error<Ek> could be more useful.

Probably yes.

> In any case expected<T, E1, ..., En>  should provide a visit() function.
> Having more than one Error makes this absolutely necessary.
> 
> When having more than one error in expected,  having access to each one
> of them using a different interface would make the user code more
> complex, isn't it?

Yes if all kinds of failure are treated as an error.

Outcome distinctly categorises failure-due-to-error and
failure-due-to-exception. I think that important given its different use
case to Expected which is explicitly for low latency and C++ disabled
environments.

But Expected as the STL type, I think your proposal is safe. Most of
your users will throw exceptions for exceptional situations.

>>>> - New typedefs checked_outcome<T>/checked_result<T>,
>>>> checked_outcome_e<T>/checked_result_e<T> are added. These mirror the
>>>> editions just described, but checks and default actions occur on all
>>>> observer usage so hidden reinterpret_cast<> never occurs. Implicit
>>>> conversion from non-checked varieties is permitted to checked
>>>> varieties,
>>>> but not the other way round.
>>>> https://github.com/ned14/boost.outcome/issues/47. Examples:
>>>>
>>>> ```
>>>> // Note result<T> implicitly converts to checked_result<T>, but not
>>>> // the other way round. So we can use same make_errored_result().
>>>> checked_result<Foo>
>>>> v(make_errored_result(std::errc::invalid_argument));
>>>> assert(v.has_error());   // true
>>>> assert(!v.has_value());  // true
>>>> // .value() throws std::system_error(
>>>> //   std::make_error_code(std::errc::invalid_argument));
>>>> Foo f(std::move(v.value()));
>>>> ```
>>>>
>>>> ```
>>>> checked_result<Foo> v(make_valued_result(Foo()));
>>>> assert(!v.has_error());  // true
>>>> assert(v.has_value());   // true
>>>> // .error() returns a default constructed (null) error_code_extended
>>>> // when result is valued to indicate "no error here"
>>>> error_code_extended ec(std::move(v.error()));
>>>> assert(!ec);             // true
>>>> ```
>>> I will need more rationale about the need of this classes and why we
>>> need to do an action while observing them. Please, could you elaborate?

To save writing boilerplate in the most common use cases.

> if error returns the stored error or a default error when valued, why we
> don't use another name for the function?

You mean other than .error()?

> Why do we want to change the signature and the semantic of an existing
> function (when I say existing, I'm referring so
> std::experimental::expected, or for ).
> 
> Why not have always the same interface and adding an error_or function
> as suggested in the Expected proposal?  This function could be generic
> as far as we have common interface.

Again, less boilerplate.

Expected, because it lets users choose their E type, unavoidably
requires more boilerplate. I don't like typing boilerplate, it makes my
fingers and wrist hurt more, it clutters my source code, hence Outcome.

> What your checked_outcome<T>::error will return if the is a exception_ptr?

Ah, finally someone brought this up. Good!

It returns an error_code with bad_outcome_category and
bad_outcome_errc::exception_present.

This is despite .has_error() being false! Now I think that this is the
right way of implementing this, but I'll straight away agree that it is
not uncontroversial to me. My argument would be that it is a magic
cookie error code to indicate you've done a logic error. Some would say,
can't you throw an exception, it is a logic error after all? That would
be my second preference if this magic cookie is felt by reviewers to be
unwise. I definitely do not think .has_error() should be true if the
outcome is exceptioned. And yes, I do get the irony here considering my
arguments about not removing the empty state.

I will tell you one thing: in all my nearly two years of using Outcome,
I have never yet seen a bad_outcome_errc::exception_present error_code
turn up. It has made it hard for me to be sure if this choice is the
right call or not.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/