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: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2017-05-28 09:35:36

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.

Le 27/05/2017 à 16:13, Niall Douglas via Boost a écrit :
>>> - outcome<T> and result<T> will have their empty state removed, and all
>>> observers gain narrow contracts. Default construction is disabled.
>>> https://github.com/ned14/boost.outcome/issues/44. Example:
>>>
>>> ```
>>> result<Foo> v(make_errored_result(std::errc::invalid_argument));
>>> assert(v.has_error());   // true
>>> assert(!v.has_value());  // true
>>> // As if Foo(reinterpret_cast<Foo &&>
>>> //  (error_code_extended(std::errc::invalid_argument));
>>> Foo f(std::move(v.value()));
>>> assert(v.has_error());   // still true
>>> assert(!v.has_value());  // still true
>>> ```
>> Just a question I could do for the existing library also. What has_error
>> mean for outcome, result? is that it has an EC or that it has no value?
> .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.

Why not name the getter error function get_error_code, .... as the 
function doesn't returns a generic Error but one of them.

I don't think using the same name for different things is helping in 
generic programming.
>
>> And now that we don't have empty, what is the sens of has_error for
> result?
>
> There will be no longer an empty state in result<T>, but for generic
> programming I'll be retaining a .has_error() so end users can more
> easily swap a result<T> for a result_e<T>.
:(

I see generic programming in other terms.
>
>> 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.
>
>>> (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.
>
>> If we had a expected<T, E1, .., En> what will be the differences between
>> outcome<T> and expected<T, error_code_extended, exception_ptr>?
> 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.

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?

> outcome<T> continues to provide:
> - T& .value()
> - error_code_extended .error()
> - std::exception_ptr .exception()
>
> i.e. hard coded.
Ok, so the single difference would be about the error observers 
returning by value and having an explicit name.
>
>>> - New typedefs outcome_e<T> and result_e<T> are identical to outcome<T>
>>> and result<T> except for adding a formal empty state. Observer contract
>>> slightly widens, an attempt to use an empty object throws a
>>> bad_outcome_access exception. Implicit conversion from non-empty-capable
>>> varieties is permitted to empty-capable varieties, but not the other way
>>> round. Default construction is to **empty**.
>>> https://github.com/ned14/boost.outcome/issues/44
>> Okay this corresponds to what others are naming optional_outcome,
>> optional_result.
> I'm not wedded to result_e<T> etc. Ok, I'll change result_e<T> and
> outcome_e<T> to optional_result<T> and optional_outcome<T>. Done at
> https://github.com/ned14/boost.outcome/issues/44
Neither me. Just wanted to name the types in a more explicit way. 
result_e doesn't tells me the intent.
>> If we had a optional_expected<T, E1, .., En>
>> what will be the differences between result_e<T> and
>> optional_expected<T, error_code_extended>?
>> what will be the differences between outcome_e<T> and
>> optional_expected<T, error_code_extended, exception_ptr>?
> I am not sure what semantics you have chosen for optional_expected<>.
optional_expected<T, ... > should be an optimize version of 
optional<expected<T, ...>>.
>
> optional_outcome<T> is exactly like outcome<T> but with an added empty
> state and a default constructor which constructs to empty. Otherwise
> identical.

>
>>> - 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?
> checked_optional_outcome<T> equals exactly the outcome<T> in the
> presented library.
>
> checked_optional_result<T> equals exactly the result<T> in the presented
> library.
>
> All the discussion of the presented library to date applies to the
> checked_*() functions.

if error returns the stored error or a default error when valued, why we 
don't use another name for the function?
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.
What your checked_outcome<T>::error will return if the is a exception_ptr?

>
>>> Still to be decided:
>>> ====================
>>> - Should *_e() varieties provide convenience .get(), .get_error(),
>>> .get_exception() which returns T, error_code_extended and
>>> std::exception_ptr by value moved from internal state, resetting state
>>> afterwards to empty? These would mirror future.get()'s single shot
>>> observers.
>> We need a valid use case to introduce them and even more as member
>> functions.
>> In any case, these functions can be defined on to of the provided
>> interface, and could be non-member functions, isn't it?
> There is an argument to make all the observers of Expected or Outcome
> free functions. It certainly would fit how std::begin() etc work. So:
>
> expected<T, E> foo(T());
>
> if(has_value(foo)) ...
>
> T x(std::move(value(foo)));
>
> T y(std::move(get(foo)));
>
> ... and so on.
>
I see a difference between the minimal interface and other functions.
You can not define has_value in function of other functions.
You could define a wide value() function using has_value() and 
deref()/operartor*.

If you go the future::get way, don't forget that you cannot set twice a 
promise.

Vicente