Hi, thanks Oliver to move the Coroutine library to a Boost review ready proposal. Before doing a formal review, I have some comments, suggestions and questions after a quick reading. Design ==== * I don't know if showing the signature in the documentation will be more informative. This will avoid, in particular, the be forced to explain what T and R and in the the current declaration. Could you replace template< typename Signature, typename Allocator = ctx::stack_allocator
class coroutine { public: class self_t { public: T yield( R); void yield_break(); }; by template< typename Signature, typename Allocator = ctx::stack_allocator
class coroutine; template< typename R, typename ...ArgTypes, typename Allocator
class coroutine<R(ArgTypes...)> { public: class self_t { public: tuple<ArgTypes...>yield( R); void yield_break(); }; * Could you add the following noexcept specifier? class coroutine { // ... coroutine() BOOST_NOEXCEPT; coroutine( coroutine && other) BOOST_NOEXCEPT; coroutine & operator=( coroutine && other) BOOST_NOEXCEPT; explicitoperator bool() const BOOST_NOEXCEPT; bool is_complete() const BOOST_NOEXCEPT; //... }; void swap( coroutine & l, coroutine & r) BOOST_NOEXCEPT; The same applies to generator. * As the template class Allocator is not used to allocate internal memory of the coroutine class. Could the Allocator parameter be renamed StackAllocator? * Could you add an standard Allocator parameter as e.g. the one of packaged_task? 0. template <class F, class Allocator> explicit packaged_task(allocator_arg_t, const Allocator& a, F&& f); 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. template<typename>class coroutine; template< typename R, typename ...ArgTypes
class coroutine<R(ArgTypes...)> { public: //... template< typename Fn,typename Allocator> generator( Fn fn, std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, Allocator alloc = Allocator() ); //... }; * Parameters and move semantics. Could the signature parameters be rvalue references? R operator()(A0 a0, ..., A9 a9); I'm asking this because I think that Boost.Tuple has not implemented yet move semantics. * 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, * Usually (at least in the C++ standard) a function doesn't throws because of its preconditions are not satisfied. Preconditions: |size| > ctx::minimum_stacksize(), |size| < ctx::maximum_stacksize() when ! ctx::is_stack_unbound(). Effects: Creates a generator which will execute |fn|. If |do_unwind| is |stack_unwind| the destructor of |*this| unwinds the stack before destructing it. If |preserve_fpu| is |true| the floating-point registers are preserved between context switches. Throws: /invalid_argument/ if a precondition is not satisfied. Should the function throw in this case? This should be coherent with |Result operator()()| <http://ok73.ok.funpic.de/boost/libs/coroutine/doc/html/coroutine/generator/generator.html#coroutine.generator.generator._code__phrase_role__identifier__result__phrase___phrase_role__keyword__operator__phrase__phrase_role__special________phrase___code_> Preconditions: |*this| is not a /not-a-generator/, |! is_complete()|. which doesn't throws any specific exception if the preconditions are not satisfied. . Documentation ========= coroutine.html s/it can migrated between threads/it can migrate between threads/ s/yield_return/yield_break/ * documentation for the following constructor is missing in the reference template< typename Fn > coroutine( Fn && fn, std::size_t size = ctx::default_stacksize(), flag_unwind_t do_unwind = stack_unwind, bool preserve_fpu = true, Allocator alloc = Allocator() ); Best, Vicente