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-06-04 08:37:25

Le 03/06/2017 à 18:27, Peter Dimov via Boost a écrit :
> Vicente J. Botet Escriba wrote:
>
>> What about the non-member function, so that we don't have code as 
>> x.template subset<E2...>() and we have subset<E2...>(x) instead?
>
> You're right about the need for .template here, but I'm not sure that 
> the need to call this function would be frequent enough to justify 
> putting it into the outer namespace (potentially std). Leaving it as a 
> member seems an acceptable compromise to me.
>
> Do you have any comments about my proposed expected<T, E...>? Overall 
> impression? Good idea? Bad, no reason to waste time with it?
It is an excellent idea to implement first a never-empty variant and 
then implement expected on top of it.

expected<T, E...> was in the list of open points of my proposal. You 
have implemented it (even if still partially)  :). I'll add a link to it.

You have taken some deviations from the proposal that could merit discussion

* missing sizeof...(E) >0 assertion
* unexpected_ type is not an explicit and different type
* inheritance of bad_expected_access<T> from from bad_expected_access<> :)
* Additional throw_on_unexpected

https://github.com/pdimov/variant2/blob/master/include/boost/variant2/expected.hpp#L174
I suspect that this is is_move_constructible

* 
https://github.com/pdimov/variant2/blob/master/include/boost/variant2/expected.hpp#L174
https://github.com/pdimov/variant2/blob/master/include/boost/variant2/expected.hpp#L334
has_error/error should be defined only if sizeof... (E) is 1
* unexpected()/error() should have a narrow contract (but this needs to 
be discussed in Toronto)
* I know that you don't want to return by reference but unexpected can 
return by reference when sizeof...(E) is 1
* remap_errors seems close to what I've called adapt
* 
https://github.com/pdimov/variant2/blob/master/include/boost/variant2/expected.hpp#L373

         return mp_with_index<mp_size<expected>>( v_.index(), [&]( auto 
I ) {

             return this->_remap_error<R>( I, f, get<I>(v_) );

});

I don't see how get<I> can work here.

The same for 
https://github.com/pdimov/variant2/blob/master/include/boost/variant2/expected.hpp#L132

Could explain me how the function parameter I is know at compile time?

* ::then seems to be the monad::bind, could you confirm?
The monadic functions were requested to be removed from the Expected 
proposal and managed in a general case.

* Missing catch_error from the previous version of the Expected proposal.
* Wondering from constructors from convertibles expected<T,E...> from 
expected<U, G...>.
* Factories are missing
* I don't know if expected<void, E...> has been implemented

Thanks for working on it.

Vicente

P.S. While we differ on some points about narrow contracts, returning by 
reference I will live with the committee decision.