Re: [boost] [next gen futures] Lightweight monad ready for inspection

Subject: Re: [boost] [next gen futures] Lightweight monad ready for inspection
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-06-20 09:08:13

On 20 Jun 2015 at 13:48, Bjorn Reese wrote:

> On 06/19/2015 07:03 PM, Niall Douglas wrote:
> 
> > https://github.com/ned14/boost.spinlock/blob/master/include/boost/spinlock/monad.hpp
> 
> This starts with the words "The world's most simple C++ monad" followed
> by 1200 lines of code... makes one scared of monads.

Firstly, thanks for the review. As I know you know, STL quality C++ 
is amazingly verbose. And I haven't implemented monad<void> yet, 
which will require doubling the existing lines of code to stamp out 
an almost identical monad<void> specialisation. I am currently 
strongly considering an automatically generated source solution to 
that.

(One could use enable_if everywhere to get a void and non-void single 
implementation, but I want minimal build time costs).

> Here are some random throughts I had while reading the code.
> 
> Minor comment, consider letting monad_error inherit from system_error
> instead of logic_error.

http://en.cppreference.com/w/cpp/thread/future_error

I agree with you, but that's the C++ standard.

Of course, monad need not duplicate future here, but I think if I 
claim "future semantics" then it ought to.

> When throwing monad_error, you cast to int (presumably to satisfy some
> compiler.) Can't you simply use make_error_code() instead? That will
> also ensure that you set the correct category.

VS2015 RC was the problem. The current cast to int workaround is the 
first thing I'll try fixing when VS2015 RTM comes out.

The category is correct though. The throw_error() function is a 
template parameter, and that is a struct detail::throw_monad_error 
which in turn throws monad_error(monad_errc). monad_error takes a 
std::error_code, and the C++ 11 STL uses the 
std::is_error_code_enum<> trait to auto convert the monad_errc enum 
into the correct error code and monad_category category.

I lifted that pattern of how to throw a future_error straight from 
Boost and two STLs I examined. It's identical to std::future<T> and 
boost::future<T>.

> Why does one of the value_storage constructors call abort() rather than
> throwing an exception?

value_storage lives in the detail namespace, and its ability to carry 
a future<T>* in its variant space is an internal implementation 
detail nor exposed to public users. The only users of that facility 
are lightweight future-promise. They (should) never ever copy 
construct a value_storage, so if a copy construction ever occurs when 
the contents are a future<T>*, that's memory corruption and/or bad 
programming by me. It was purely defensive programming on my part.

> Why does monad::get_error() throw on no_state, rather than simply
> returning an error_code that says so?

A very good question. I purely did it that way round for these two 
reasons:

1. Any attempt to get any state from an empty monad always throws 
no_state consistenty no matter what you call.

2. future-promise always throws a no_state if either has no state, so 
boost::future<T>::get_exception_ptr() will throw a no_state instead 
of returning an exception_ptr of no_state. monad<T> matches that 
behaviour.

Actually, I just lied. The monad get_or(), get_error_or(), 
get_exception_or() functions never throw no_state, they return the OR 
value. I am unsure if that is a wise design choice.

There is also another potential design problem in that get_error() 
returns a null error_code if the monad is not errored even if it is 
excepted. I did it that way to match get_exception() as 
boost::future<T>::get_exception_ptr() returns a null exception_ptr if 
there is no exception. However there is a discrepency, because if the 
monad is errored not excepted and you call get_exception(), you'll 
get an exception_ptr to the system_error for the error_code, and it's 
not the same the other way round.

There is a logic to this though. monad<T> is effectively a tribool 
state of empty/value/excepted where an errored state always converts 
when needed into an exception state, but never the other way round. 
The errored state is therefore a non-type-erased excepted state, 
while the excepted state is a type-erased state. I am just unsure if 
that is a wise design.

> Is there any difference between monad::get() and monad::value()? If
> they are synonyms, then you may consider removing monad::get() to keep
> the class interface smaller, and instead overload std/boost::get().

They are identical.

I hate multiple names for the same function. However, Expected<T, 
E>'s value fetcher function is called value(), and I was hoping for 
an easy swap for code using Expected<T, E>.

That said, if there is any opposition here I will gladly remove 
value() and leave it with just get().

Niall

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