[Review][Move] Boost.Move Review Results
---------------------------- Executive Summary ---------------------------- Ion GaztaƱagas' Boost.Move library was reviewed from May 10th, 2010 through May 24th, 2010. The original review manager was OvermindDL1 who later went missing. Eleven individuals joined the discussion with the author. Six reviews were submitted: - 5 reviews ACCEPT - 1 review CONDITIONALLY ACCEPT Ion indicated he would meet the "conditional requirements" which easily satisfies a community vote of: Boost.Move library ACCEPTED ---------------------------- Overview ---------------------------- The Boost.Move library is a highly anticipated addition to the Boost collection. The primary motivation is to introduce usable move functionality in "a portable syntax between C++0x and C++03 compilers"; however, as Vicente Botet articulated, "Most people think that this is an emulation library, but it is more than that as it implements most of the C++0x Move semantics classes and functions." Reviewers' comments: - "This library is long overdue!" [Terry Golubiewski] - "The tutorial is well written." [Vicente Botet] - "Compared to my adobe based move implementation, I think yours is superior." [Daniel James] - "There is a lot of ingenuity in making the move emulation work, and Ion has made it very accessible and managed to make differences between C++03 and C++0x vanish behind macros and free functions." [Jeff Hellrung] An active discussion included: - exposing boost::rv<T> publicly - The merits of fully defining requirements of boost::rv<T>& - Duplicated functionality found in MPL and Type Traits - The necessity for two emulation modes - The merits of macros - Library author usage of Boost.Move - Non-throwing move constructors - Various potentially useful type traits - Other move implementations While it is unfortunate that a timely review result was not published, it did provide an opportunity to discuss some of the concerns raised in the review with actual users of the library. One such concern was the "two emulation modes." At some point in the discussions it became classified as "Safe vs. Fast". When coined with that bias "safe" would be the preference; however, further investigation and inquiries of actual users showed the "fast" version is perfectly acceptable as the default. Additionally, "safe" is a misnomer considering that one mode is not any safer than the other. Each mode has benefits and limitations as enumerated by Ion in the documentation and review responses. The past six months of use would indicate that defaulting to the "fast" version, as preferred by Ion, is in practice a good decision. Thank you Ion for this well thought out and excellent submittal. Thank you reviewers for expending time and energy to further Boost. Without those who are dedicated to review submitted libraries, the entire process and Boost itself falls apart. The community will benefit from the effort each of you put into this library. Best regards - Michael Caisse ---------------------------- Summary of changes ---------------------------- The following changes are planned based on discussions from the review: -- Conditional Acceptance -- 1. "Two Emulation Mode" documentation typos must be resolved. The descriptions, requirements and limitations need to be perfectly clear for the end user. -- Other Issues -- 1. The class rv should be public and documented. 2. is_movable should be renamed and the documentation clarified. It should only be enabled in C++03. A possible alternative name is has_move_emulation_enabled. 3. Nearly all reviewers voiced a preference that the move library should use existing functionality from MPL and TypeTraits libraries. While on one hand this creates a dependency between move and other libraries it results in reuse of accepted and well tested libraries. If a non-coupling option is also provided, it should not be the default. 4. Additional traits discussed. Resolution: add to TypeTraits library as actual use-cases are found. 5. Provide a boost/move.hpp that includes boost/move/move.hpp 6. Document the interaction of the two emulation modes. 7. Include example for making a container "movable". 8. Two emulation modes - Ion was hopeful that reviewers would have a uniform opinion that would result in a single mode. The trade-offs identified by Ion in the documentation were apparent to all reviewers; however, different conclusions were drawn and consensus was not met. The granularity of control was a common issue with nearly all reviewers. Therefore, keep both emulation modes with Optimized as the default and the granularity of control at or more "local level" than the macro provides (class level). 9. emulation_limitations.html: s/overaloading/overloading/ 10. Fully qualify enable_if ( ::boost::emable_if ) 11. Optimized version of forward should use identity so it will not try to deduce T. 12. Make sure required headers are mentioned for tutorials. 13. is_class in is_movable template< class T> struct is_movable : mpl::and_< is_class<T>, mpl::not_< is_rvalue_reference<T> >, is_convertible< T, rv<T>& > > {}; - or - another solution to is_movable problems with references. 14. On throwing move constructors, Ion should follow as close to the standard's behaviour as possible. This can be revisited after the initial release since it is a performance optimization. -- Michael Caisse Object Modeling Designs www.objectmodelingdesigns.com
participants (1)
-
Michael Caisse