Oliver Kowalke wrote:
I dislike the namespace. "coro" is meaningless and awkward.
STL's namespace is 'std' I tought it's ok if I do it liekwise.
Should your library be standardized, it may appear in the std namespace or some subnamespace which could well be std::coro. However, Boost has policies on namespaces and "coro" doesn't fit.
The examples are appropriate for tutorial purposes, but there is a complete lack of non-trivial examples to illustrate how the library can be employed. What can one do with this library that isn't possible otherwise? What is better, according to any number of criteria like less code, faster, etc., using this library?
sub-directory contains 'example' contains code demonstrating how to use a coroutine together with an streambuf in order to provide an non-blocking istream. The stream reads data from a socket demultiplexed by boost::asio::io_service. Might this be 'non-trivial'?
I didn't even look under libs to see the examples until you prompted me. However, I don't see that example. Nevertheless, it sounds interesting.
I think the documentation should describe simple examples and the 'example' sub-directory contains more complex ones (but harder to describe).
All of the examples, or most if there are a great many, should be described in the documentation, with links to the full source. This is true for two reasons. First, those reading the documentation are not looking through the source tree. Second, the documentation affords the opportunity to discuss the examples freely, giving an introductory explanation as well as dissecting and discussing interesting parts of each. As there are no comments in the examples, documentation is even more important. Even if you added comments, which is reasonable, the examples should be introduced, at least, in the documentation.
How much of the interface remains what was designed by Giovanni Piero Deretta? Given all of the changes during the review, I wonder how much remains.
After looking over the suggested modifications It seams that not very much of Giovanni's design will remain (self_t will be removed from coroutine-fn signature).
Your mention of his design should be moved to the Acknowledgements section, then.
The "Exceptions in coroutine-function" section of coroutine.html, and the corresponding section of generator.html, suggests that exceptions must be thrown using Boost.Exception. Is that the intent? If so, you need to make that point clear.
It should, if you do not follow the recommendations in boost.exception the exceptions thrown inside the coroutine-fn will be re-thrown with type 'unknown_exception'.
You need to be specific about those points, then.
I suppose that isn't true in C++11.
I don't know. At least for compilers no implementing C++11 it should be true.
If you missed it, I'm referring to std::exception_ptr, std::current_exception(), and std::rethrow_exception().
Also, what is the namespace of forced_unwind? This section is the first mention of it, so one is left to wonder if it belongs to the library or > not.
'forced_unwind' is in 'detail' namespace. The user should not use it because the exception is used to unwind the stack.
This is confusing then. You reference the exception type by name, illustrate catching it by name, as well as having a catch-all handler, thereby suggesting that users can write that same set of handlers, but the type name is in the detail namespace, so users shouldn't reference it. If users shouldn't know the name, then you shouldn't mention it. Instead, you should tell them they mustn't absorb unknown exceptions because they'll absorb an important exception used by the coroutine implementation. Thus, you should illustrate what's acceptable by: try { // coroutine-using code } catch (user-known-exception-type const & e) { // rethrow or absorb e } catch (...) { // react to exception throw; // rarely good to absorb unknowns } If you intend that they should be able to differentiate between your exception and others, then the name shouldn't be in the detail namespace and you should show the following: try { // coroutine-using code } catch (boost::coroutines::forced_unwind const &) { throw; } catch (user-known-exception-type const & e) { // rethrow or absorb e } catch (...) { // react to exception throw; // rarely good to absorb unknowns }
Also, the note, in those sections, references "ABIs", but gives no references and fails to specify which ABIs are meant.
Should I not expect that the user knows what an ABI is and which ABI is used on its platform?
I didn't mean to suggest you needed to define ABI, though a link to Wikipedia or similar location wouldn't hurt. Instead, my concern is that you refer to "ABIs" as if to say all extant ABIs. I doubt you've studied them all, so that's too broad. Perhaps you mean, "the ABIs on all supported platforms".
Should I add descriptions of all available ABIs?
No, but links to details wouldn't hurt if practicable. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com ________________________________ IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.