Boost logo

Boost :

Subject: [boost] [Review.Coroutine] More comments, questions and suggestions
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-09 14:02:35


I understand Oliver that you are busy as all of us and that it is better
to provide first the minimal scope you are confident with. I expect
however that you will add symmetric coroutines based on the work done by
Giovanni in a near future. BTW, adding a link to the Giovanni
documentation will be welcome.

I have more questions concerning the interface before finishing my review:

* How the library behaves when the coroutine parameters are const&

typedef boost::coro::coroutine< int( AType const& ) > coro_t;

int fn( coro_t::self_t & self, AType const& v)
     ???j = self.yield( v.get_int() );
     /// ...
What should be the type (???) of j? Does AType const& works?

* Have you tested with other kind of parameters? arrays, pointers, ...
It would be great if the documentation includes some examples, with the
different types that can be used.

* The way the parameters of the coroutine call are retrieved the second
time is a little bit disturbing at a first glance, but once you
understand how this works, it is not so complex (This applies to the
library under review and the original one, as both share the same design).

I don't know if the the self object couldn't store the parameter
binding, so that the user will not be concerned by the yield result type.

typedef boost::coro::coroutine< int( int) > coro_t;

int fn( coro_t::self_t & self, int i)
     // binds current parameters to the self object. Done only once.
     self.bind(i); // ***

     std::cout << "fn(): local variable i ==" << i << std::endl;

     // save current coroutine context
     // transfer execution control back to caller
     // no need to assign the result. It is up to yield to fill the
binded parameters.
     self.yield(i); // OTHERS ***
     // i == 10 because c( 10) in main() and i has been bound in self.
     std::cout << "fn(): local variable i ==" << i<< std::endl;

     return i; // LAST

I don't know this bind design introduce more problems that it could
solve. I don't know neither if this could degrade the performances.

* In the preceding example there is an asymmetry between the value
returned the last time and the others.

     self.yield( i); // OTHERS
     return i; // LAST

What about forcing the coroutine function return void, as it is the case
now for the generator?

voidfn( coro_t::self_t & self, int i)
     std::cout << "fn(): local variable i ==" << i << std::endl;

     std::cout << "fn(): local variable i ==" << i<< std::endl;


This should help to simplify user code, isn't it?

* I don't know if you have reached to make the StackAllocator standard
compliant, but if this is the case you could add also a specializations
of the use_allocator template so that Boost.Container can take it in


Boost list run by bdawes at, gregod at, cpdaniel at, john at