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