Re: [Boost-users] [Review.Coroutine] Coroutine review comments

Subject: Re: [Boost-users] [Review.Coroutine] Coroutine review comments
From: Oliver Kowalke (oliver.kowalke_at_[hidden])
Date: 2012-09-08 15:09:54

Hello,

Am 07.09.2012 23:35, schrieb Nat Linden:
> My intention was to test Oliver's Coroutine proposal by replacing it
> into our code, but this is not possible without something equivalent
> to 'future'. Accordingly, this review is based on the library
> documentation rather than on actual usage.(One could imagine passing a coroutine object itself as a callback for
> some other library, then calling yield() to await that callback. There
> are two obstacles. One is that I know of no way to obtain, within the
> coroutine-function, a reference to the containing coroutine object.
> The initial 'self' parameter is not that object; it presents a
> different API. A more serious problem is that the value returned by
> yield() is fixed by the coroutine template's Signature parameter. With
> futures, each future object can return a different type; a given
> coroutine can instantiate an arbitrary number of future objects with
> an arbitrary number of types to await an arbitrary sequence of
> callbacks.)
I decided not to provide the future<> facility becuase I've had some 
concerns about the 'correctness' of the interface.

My intention is to provide a small, clean interface which is hardly to 
misuse.
In my opinion a coroutine is a language-level construct allowing to 
enter a routine multiple times (by preserving the local state in the 
routine) == multi-entry routine. Therefore I want a strong contract 
between the caller and the coroutine.
This contract is established via the signature and the return type of 
the coroutine (coroutien-fn).
Thus the interface provided by boost.coroutine must be stringent in the 
case that the coroutine can only activated/entered via 
coroutine<>::operator() - no other way to jump into the coroutines body 
(coroutine-fn) is possible. Additionally the return value and the 
coroutine parameter of coroutine<>::operator() must be the same as 
declared in the signature.

The future<> concept in Giovannis library violates this contract. You 
have additional ways to jump into the coroutine's body. You can pass 
other parameters and return a different type than the coroutine 
signature defines.
I think this violates the design.
With future<> you have to test if the coroutine is  waiting (call 
coroutine<>::wait()) after return from coroutine<>::operator() (context 
switch) - otherwise you can get garbage in the retur nvalue because the 
coroutine was left via future<> instead of the usual way which is 
coroutine::self_t::yield().

I don't know your code but if you need 'future' semantics I suggest 
boost.fiber (http://ok73.ok.funpic.de/boost/libs/fiber/doc/html/, 
http://ok73.ok.funpic.de/boost.fiber.zip). boost.fiber provides 
lightweight threads using boost.context (context switching). The lib 
provides classes like mutex, condition-variables, event-variables and  
futures. You can use it like boost.thread - but it provides cooperative 
multitasking.

> "The maximum number of arguments of coroutine-function is 10."
it's a limitation of boost.tuple

<snip>
I'll update the docu as you suggested.

> "An exception thrown inside coroutine-function (transfered via
> exception-pointer - see Boost.Exception for details and requirements)
> will be re-thrown by coroutine<>::operator()()."
>
> I'm not familiar with Boost.Exception. Will any exception be
> re-thrown, including exceptions raised by the runtime? Or does this
> only apply to exceptions derived from a particular base class, or
> thrown in a particular way? If there are such limitations, mentioning
> them briefly here would be extremely useful.
I gave the hint that the docu of boost.exception will describe all the 
requirements.
Usally the used should not let an exceptio nescape from a coroutine-fn.
The implementation (trampoline function inside) catches with ellipses 
and assigns it to exception ptr:
catch (...)
{
         context->flags_ |= flag_has_exception;
         context->except_ = current_exception();
}
In the worst case you get an exception of type unknown_exception rethrown.

> "Code executed by coroutine must not prevent the propagation of the
> forced_unwind exception. Absorbing that exception will cause stack
> unwinding to fail. Thus, any code that catches all exceptions must
> rethrow the pending exception."
>
> This and the example should clarify that we're talking about code
> within the coroutine-function.
OK
> (And is forced_unwind also within the boost::coro namespace?)
boost::coro::detail - not derived from std::exception
> If forced_unwind is the exception thrown when a (! is_complete())
> coroutine object is destroyed, that's worth mentioning. It's possible
> that a coroutine-function might want to perform cleanup specific to
> that case.
OK
> Example in section "Generator":
>
> Undefined make_generator() function wants a comment. Again, this is a
> hypothetical user function rather than part of the
> boost::coro::generator API.
>
> "The first argument of generator-function must be of type
> generator<>::self_t, ..."
>
> The "only" argument?
You could bind parameters to function entry
> I'd like to see some words when generator is first introduced
> clarifying the ways in which it differs from coroutine. This what I
> think:
>
> * A generator-function accepts no arguments (other than self_t), so
> yield() always returns void.
yes - it is required to return void, must except self_t as first arg and 
parameters can be bound via boost::bind()
> * yield_break() does not throw an exception in the invoking coroutine.
yield_break() was removed from the generator<>::self_t class (in git repo)

<snip>   I'll add your comments to the docu
> "generator-function is invoked the first time inside the constructor
> of generator."
>
> That makes no sense to me. Is the value passed to the first yield()
> call simply discarded? If so, why?
I tried to express that a generator must be tested before you can use it:

gen_t gen(...);
if ( gen) {
    int x = gen();
}

in order to know if gen is valid (== it will return a value) the 
return-value must be fetched from the generator-fn.

> "If generator-function can not return valid values anymore
> generator<>::self_t::yield_break() should be called. This function
> returns the execution control back to the caller and sets the
> generator to be complete (is_complete() returns true)."
>
> is_complete() is not documented for boost::coro::generator.
yes - you have to use generator<>::unspec_bool() or generator<>::operator!()
> Is there any semantic difference between executing 'return' in a
> generator-function and calling yield_break() in a generator-function?
> If not, is yield_break() provided only for consistency with coroutine?
yield_break() is removed now

regards,
Oliver