Boost logo

Boost :

Subject: Re: [boost] [Review.Coroutine] Some comments
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2012-09-05 07:25:32

Le 05/09/12 08:56, Oliver Kowalke a écrit :
>> * Could you add an standard Allocator parameter as e.g. the one of
>> packaged_task?
>> C++ International Standard
>> * I understand that the Boost.Context Allocator parameter is part od the
>> class, as context is a low-level class, but coroutine is more high
>> level. Is there a possibility to type erase the StackAllocator (and
>> preceding Allocator) template parameter, as packaged_task do? This will
>> allow to store coroutines (and generators) with the same signature in a
>> container even if they don't share the Allocator.
> You mean coroutine would have two allocators - one for allocating internal memory and one for allocating the stack?
> Removing the StackAllocator from the template arguments of coroutine should not that problem (would require an internal pointer to an internal interface - maybe detail::context_exec is a candidate).
> I've some concerns about having two allocator objects in coroutine's ctor
> .
I'm suggesting to add a std Allocator as I think the StackAllocator is
not a model refinement of std Allocator. Am I wrong?

>> * Parameters and move semantics. Could the signature parameters be
>> rvalue references?
>> R operator()(A0 a0, ..., A9 a9);
> Hmmm - this would require a second version of opertor() and all arguments are moveable?!
> R operator()(BOOST_RV_REF( A0) a0, ..., BOOST_RV_REF( A)) a9);
I'm not sure. For example, the thread constructor is

     template <class F, class ...Args> explicit thread(F&& f, Args&&...

and the function F shouldn't define several overloads. I have not
installed the lib. Maybe you can give a try with a MovableOnly argument
and see how it behaves.
>> I'm asking this because I think that Boost.Tuple has not implemented yet
>> move semantics.
> don't know - would that be an issue.
I think so, because the yield function returns a tuple with the new
ArgTypes... which should be moved not copied.

         tuple<ArgTypes...>yield( R);

If ArgTypes is MovableOnly, this will not compile. I don't know if
fusion::tuple support already Move semantic for their args.
>> * What about moving some of the coroutine/generator optional parameters
>> to a coro::attributes class
>> std::size_t size = ctx::default_stacksize(),
>> flag_unwind_t do_unwind = stack_unwind,
>> bool preserve_fpu = true,
> instead of passing those threee params as args to coroutine ctor? good point
>> * Usually (at least in the C++ standard) a function doesn't throws
>> because of its preconditions are not satisfied.
> then assert?
Yes, I think.
> pre-conditions should be checked but how should I react if I get garbage from the user?
You could use assert.
>> This should be coherent with
>> |Result operator()()|
>> Preconditions:
>> |*this| is not a /not-a-generator/, |! is_complete()|.
>> which doesn't throws any specific exception if the preconditions are not
>> satisfied.
> does an assert - OK the I'll replace throwing excpetions be assertions
> I think it will be easy to incorporate your suggestions - but during
> the boost.context review I was told not to modify the reviewd library
> (if the review is still on-going).
Well, as you are using Git it should be easy to create a branch, isn't
it? ;-)


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