[boost] Comments on Boost Any

From: Kevin S. Van Horn (Kevin.VanHorn_at_[hidden])
Date: 2002-11-14 13:07:10

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.