Re: [boost] [Review.Coroutine] More comments, questions and suggestions

Subject: Re: [boost] [Review.Coroutine] More comments, questions and suggestions
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-10 18:30:26

Le 09/09/12 20:21, Oliver Kowalke a écrit :
> Am 09.09.2012 20:02, schrieb Vicente J. Botet Escriba:

> * 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).
> your mean from self_t::yield()?
Yes.
>>
>> 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.
> don't know - I could add it but it seams to me syntax sugar
Maybe, but  not having to store explicitly the new parameter values 
seems a good improvement, if doable.
In addition the implementation could use it to store directly the actual 
parameters, instead of needing to store them in the self_t instance and 
them returning them when yield is called.

Thoughts?
>>
>> * 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)
>> {
>>     self.bind(i);
>>     std::cout  <<  "fn(): local variable i =="  << i <<  std::endl;
>>
>>     self.yield(i);
>>     std::cout  <<  "fn(): local variable i =="  << i<<  std::endl;
>>     self.yield(i);
>>
>> }
>>
>> This should help to simplify user code, isn't it?
> I would not force the user to use self_t::yield() a simple return 
> statement does the same.
The current interface forces the user to return the last value using 
'return', which seems no better to me. Or, would this perform better?

BTW,  I understand that the following coroutine returns twice, but it is 
a little bit obscure, as there are two ways to return a value from a 
coroutine.

int f( coro_ref::self_t & self, int a)
{
     return self.yield( a);
}

LUA follows the same schema, but I don't share it. I you adopt the same 
design as generators there will be no possible confusion. Only one way 
to yield a value.

What do others think?

> I don't know if it would be a better interface design - I'm uncertain 
> in this point.
> in the case of generators - the generators have no signature but 
> coroutines do.
>
>>
>> * 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 account.
> StackAllocator is not the same as the allocators known from the STL.
> StackAllocator return the address from the top or bottom of the 
> allocated memory block - depending if the stack grows downwards or 
> upwards.
> Anyway - it's  an issue of boost.context.
>
Sorry. I believed you had reached to mix them. Maybe you could reach to 
make a StackAllocator model a standard allocator just by renaming the 
stack allocator functions. This will break existing code defining its 
own stack allocator but the change seems minor to me.

Best,
Vicente