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