Re: [boost] [Review.Coroutine] Vicente's review

Subject: Re: [boost] [Review.Coroutine] Vicente's review
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-13 12:43:26

Le 13/09/12 17:38, Oliver Kowalke a écrit :
> Am 13.09.2012 16:42, schrieb Vicente Botet:
>>
>>>
>>> generator< int > gen( f);
>>> if ( optional< int > val = gen() ) {...}
>>>
>>> if the generator function simple returns (no yield() called) then gen()
>>> returns a none (== invalid/unset optional).
>>>
>> I don't think optional should be used. The user must know if it can 
>> call the
>> generator to get the next value or not.
> That was my previous design. The user has to check the generator like:
>
> if ( gen) before asking the next value
>
> if ( gen) {
>    int x = gen();
>    ...
> }
>
> This design required pre-fetching and storing the return value via a 
> context jump into the generator routine.
> Only with this mechanism I was able to know if the next call to 
> generator<>::operator() will return a value.
> (requires that the first pre-fetch is done in the ctor of generator<>
>
> The difference to the actual design
>
> if ( optional< int > val = gen() ) {
>    ....
> }
>
> is that I don't need to pre-fetch and not to store the optional<> as 
> parameter (optional is only created inside generator<>::operator()).
>
Why did you stored the pre fetched value in an optional<>l in the old 
design? Storing it in R seems enough to me, and with move semantics, 
moving it.
>>
>> My point was independent of the generator interface. I don't think the
>> asymmetry is beneficial, but maybe I'm missing something important.
>>
>>
>>
>>> In the case of coroutines coroutine<R>::operator() should still 
>>> return R
>>> (not optional<R>).
>>>
>>> void f( self_t &) {}
>>> coroutine< string( int, int) > coro( f);
>>> string s = coro( 1, 2);
>>>
>>> Which value should coroutine< string >::operator() return if the
>>> coroutine-function is required to return void and the user simply 
>>> returns
>>> without calling yield()?
>>>
>> A coroutine should yield a value once it is called/resumed. The 
>> library can
>> check that a value has been yield when the function returns, isn't it?
>
> If the coroutine function does not call self_t::yield() at all no 
> return value can be fetched and returned to the caller.
> That means in the corrected example above:
>
> void f( self_t &, int x, int y)
> {}
> coroutine < string( int, int) > coro( f);
> string s = coro( 1, 2);  // f did not return a value, what value 
> should s have after this?
>
> 's' would contain garbage - therefore I think the user must be forced 
> to return an value at least at the final 'return' statement.
>
>>> My impression is that this asymmetry between coroutines and generators
>>> should be kept.
>>>
>> I would like to be convinced. For the time been I'm not.
> I hope with my argument above you are convinced now ;)
Yes, I am. Ensuring that the coroutine returns one argument before 
completing is a good think.
>>>> * The access to the actual coroutine parameters is asymmetric, letting
>>>> access to old actual parameters that can point to objects that have
>>>> already
>>>> been destroyed.
>>>>
>>>> Maybe the self_t object could take care of this (or 
>>>> this_coroutine).  We
>>>> can use a get<> function to give access to the actual parameters.
>>>>
>>>>       int f20(coro_t::self_t  &  self)
>>>>       {
>>>>           self.yield(2*self.get<0>());
>>>>       }
>>>>
>>>>
>>>>       int f20()
>>>>       {
>>>>     typedef this_coroutine&lt;int(int)&gt; this_coro;
>>>>           this_coro::yield(2*this_coro::get<0>());
>>>>       }
>>> interesting idea, I find it better than the version using bind 
>>> because you
>>> don't can't forget to 'bind'. you are always required to use
>>> self_t::get<>().
>
> Would not prevent user from doing something like this:
>
> in f20( coro_t::self_t & s, int x, int y)
> {
>    s.yield( s.get< 0 > * x + y);
> }
>
> coroutine< int(int) > coro( bind( f20, _1, _2, 2, 3) );

This should not compile, as the int parameter of the coroutine signature 
shouldn't appear any more on the coroutine function.

You should do

coroutine< int(int) > coro( bind( f20, _1, 2, 3) );

> int x = coro( 5); // x == 13
>
> ? don't know which interface would be small and hardly to misuse ?
>
The user can use bind to get a function that has the signature 
int(coro_t::self_t &).

Best,
Vcente