These comments apply to release 1.29.0. Documentation and specification comments ----------------------------------------
1. Why does any_cast<V>(x) return V instead of V & or V const &? This forces me to make a copy when I may not need one. 2. The "Synopsis" in the documentation omits the pointer versions of any_cast. 3. The ValueType requirements are unnecessarily strong. The second requirement states: "A ValueType is optionally Assignable [23.1]. The strong exception safety guarantee is required for all forms of assignment." The implementation never calls an assignment operator on the ValueType, however. Perhaps you meant to say that assignment to an object of type boost::any satisfies the strong exception safety guarantee. (Requirements are conditions that users must ensure; guarantees are promises that implementers make.) 4. Minor grammatical nit in the documentation, under "ValueType requirements": "As the emphasis of a value lies in its state not its identity, values..." should be "As the emphasis of a value lies in its state, not its identity, values..." (comma added). 5. Documentation, under "Modifiers": any & swap(any & rhs); Non-throwing exchange of the contents of *this and rhs. This fails to specify what reference is returned. I suggest adding, "Returns rhs." 6. any_cast throws a bad_any_cast exception if the wrong type is given. There are, however, two cases to be distinguished here: a. The programmer anticipates the possibility of a type mismatch -- the program logic does not, and is not intended to, rule out the possibility -- and wishes to handle that case separately, in a catch clause. b. The programmer believes that the program logic guarantees that there cannot be a type mismatch. Any type mismatch is, in fact, a bug in the program. The current behavior is appropriate for case (a), but not for case (b). For case (b) something like the proposed Boost Assert would be preferable: programmer can choose, via preprocessor symbols, to have the program crash and core dump (to aid debugging), or to throw an exception giving the file and line number where the error was detected, or to simply not check at all (performance over safety). At the end of this message I've included a patch that provides for case (b), via a function known_any_cast<Type>(). Implementation comments ----------------------- 6. The copy assignment operator is implemented as any & operator=(const any & rhs) { any(rhs).swap(*this); return *this; } Andrei's comments about avoid unnecessary copying apply here: the alternative any & operator=(any rhs) { rhs.swap(*this); return *this; } avoids a copy when the right operand of assignment is an unnamed temporary, and otherwise does the same number of copies as the existing implementation. Note, however, that the value assignment template template<typename ValueType> any & operator=(const ValueType & rhs) { any(rhs).swap(*this); return *this; } should remain just as it is; copying rhs within the body of the ctor (in the any ctor) is unavoidable. 7. In the following lines template<typename ValueType> ValueType * any_cast(any * operand) { return operand && operand->type() == typeid(ValueType) ? &static_cast<any::holder<ValueType> *>(operand->content)->held : 0; } the unary "&" preceding "static_cast..." should be replaced by boost::addressof, in case an overload of operator&() is defined for ValueType. ============================================= Context diff for patch to add known_any_cast<Type>(): *** /opt/include/boost/any.hpp Sun Jan 20 13:09:43 2002 --- any.hpp Thu Nov 14 11:59:47 2002 *************** *** 11,17 **** #include <algorithm> #include <typeinfo> ! #include "boost/config.hpp" namespace boost { --- 11,19 ---- #include <algorithm> #include <typeinfo> ! #include <boost/config.hpp> ! #include <boost/utility.hpp> ! #include <boost/assert.hpp> namespace boost { *************** *** 173,178 **** --- 175,202 ---- return *result; } + + template<typename ValueType> + ValueType * known_any_cast(any * operand) + { + BOOST_ASSERT(operand && operand->type() == typeid(ValueType)); + return boost::addressof( + static_cast<any::holder<ValueType> *>(operand->content)->held + ); + } + + template<typename ValueType> + const ValueType * known_any_cast(const any * operand) + { + return known_any_cast<ValueType>(const_cast<any *>(operand)); + } + + template<typename ValueType> + ValueType known_any_cast(const any & operand) + { + return *known_any_cast<ValueType>(&operand); + } + } // Copyright Kevlin Henney, 2000, 2001, 2002. All rights reserved. _______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost