[boost] optional and typed_in_place_factory

From: James Pelletier (jamesp_at_[hidden])
Date: 2008-01-30 13:44:20

I've run into problems using the typed_in_place_factory and optional
objects.

First, I ran into a problem where I initialized a typed_in_place_factory
with a temporary and then used the factory after the temporary was
destroyed. After a few hours of hair pulling, I figured out that the
factory currently only stores references. This was a surprise because I
have been using boost::bind for quite a while and it holds copies of the
arguments. I propose that the typed_in_place_factory hold copies of the
data as well. Users can pass boost::ref objects for types that aren't
copyable with the added benefit that they explicitly know they need to
keep the referenced object alive while the factory exists. Consistency
between the behavior of boost::bind and the typed_in_place_factory is
desirable because of their similar nature.

Second, I've noticed that operator= for the optional takes a factory
object by value. It seems like taking the argument by const reference
would be desirable to prevent copying of the factory (which could be
expensive if the changes to the typed_in_place_factory mentioned
previously are accepted). Also, the conversion constructor takes the
factory by const reference, so this may have just been a simple oversight.

Below is a patch based on HEAD as of 2007-01-29.

--------------------------------------------------------------

Index: boost/optional/optional.hpp
===================================================================
RCS file: /cvsroot/boost/boost/boost/optional/optional.hpp,v
retrieving revision 1.12
diff -p -r1.12 optional.hpp
*** boost/optional/optional.hpp 26 Jun 2006 18:01:36 -0000      1.12
--- boost/optional/optional.hpp 29 Jan 2008 18:52:11 -0000
*************** class optional : public optional_detail:
*** 510,516 ****
       // Assigns from an expression. See corresponding constructor.
       // Basic Guarantee: If the resolved T ctor throws, this is left
UNINITIALIZED
       template<class Expr>
!     optional& operator= ( Expr expr )
         {
           this->assign_expr(expr,&expr);
           return *this ;
--- 510,516 ----
       // Assigns from an expression. See corresponding constructor.
       // Basic Guarantee: If the resolved T ctor throws, this is left
UNINITIALIZED
       template<class Expr>
!     optional& operator= ( Expr const& expr )
         {
           this->assign_expr(expr,&expr);
           return *this ;
Index: boost/utility/detail/in_place_factory_prefix.hpp
===================================================================
RCS file:
/cvsroot/boost/boost/boost/utility/detail/in_place_factory_prefix.hpp,v
retrieving revision 1.2
diff -p -r1.2 in_place_factory_prefix.hpp
*** boost/utility/detail/in_place_factory_prefix.hpp    26 Jun 2007
23:07:25 -0000      1.2
--- boost/utility/detail/in_place_factory_prefix.hpp    29 Jan 2008
18:52:13 -0000
***************
*** 26,32 ****
   #include <boost/preprocessor/repetition/enum_trailing_params.hpp>

   #define BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT(z,n,_)
BOOST_PP_CAT(m_a,n) BOOST_PP_LPAREN() BOOST_PP_CAT(a,n) BOOST_PP_RPAREN()
! #define BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL(z,n,_)
BOOST_PP_CAT(A,n) const& BOOST_PP_CAT(m_a,n);

   #define BOOST_MAX_INPLACE_FACTORY_ARITY 10
--- 26,32 ----
   #include <boost/preprocessor/repetition/enum_trailing_params.hpp>

   #define BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT(z,n,_)
BOOST_PP_CAT(m_a,n) BOOST_PP_LPAREN() BOOST_PP_CAT(a,n) BOOST_PP_RPAREN()
! #define BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL(z,n,_)
BOOST_PP_CAT(A,n) BOOST_PP_CAT(m_a,n);

   #define BOOST_MAX_INPLACE_FACTORY_ARITY 10