[in_place_factory] some suggestions for improvement
Hi, in_place_factory can be quite useful, as I found out recently. However, here are some suggestions to make it event better: 1. add a construction mechanism for arrays 2. add a note to the docs about deduction and using boost::ref 3. have the static 'apply' member function return the pointer to the constructed object - just like the placement new operator 4. use file iteration instead of BOOST_PP_REPEAT for maintainability Regards, Tobias
Hello Tobias, Tuesday, April 3, 2007, 9:02:45 PM, you wrote:
Hi,
in_place_factory can be quite useful, as I found out recently. However, here are some suggestions to make it event better:
1. add a construction mechanism for arrays
2. add a note to the docs about deduction and using boost::ref
3. have the static 'apply' member function return the pointer to the constructed object - just like the placement new operator
4. use file iteration instead of BOOST_PP_REPEAT for maintainability
If I may, I'd like to add: 5. add in_place_factory and typed_in_place_factory for zero constructor arguments. I remember someone mentioned this before, but I can't remember any one answered. -- Best regards, Andrey mailto:andysem@mail.ru
Andrey Semashev wrote:
Hello Tobias,
Tuesday, April 3, 2007, 9:02:45 PM, you wrote:
Hi,
in_place_factory can be quite useful, as I found out recently. However, here are some suggestions to make it event better:
1. add a construction mechanism for arrays
2. add a note to the docs about deduction and using boost::ref
3. have the static 'apply' member function return the pointer to the constructed object - just like the placement new operator
4. use file iteration instead of BOOST_PP_REPEAT for maintainability
If I may, I'd like to add:
5. add in_place_factory and typed_in_place_factory for zero constructor arguments.
Quick discussion: Often the reason against nullary function overloads is the earlier point of instantiation (nullary functions can't be templates), but that shouldn't be the problem here because 'in_place_factory's nested 'apply' is a template and 'typed_in_place_factory' is complete enough to be instantiated. For the 'inline in_place_factory0 in_place()' function the explicit 'inline' keyword is needed, however, not to break the ODR. OK. It seems reasonable and it seems there are no unsolvable problems around, so let's add it ;-).
I remember someone mentioned this before, but I can't remember any one answered.
That probably means that there's no maintainer. If that's so, I'd pick up this role. Regards, Tobias
Tobias Schwinger wrote:
Andrey Semashev wrote:
Hello Tobias,
Tuesday, April 3, 2007, 9:02:45 PM, you wrote:
Hi, in_place_factory can be quite useful, as I found out recently. However, here are some suggestions to make it event better: 1. add a construction mechanism for arrays 2. add a note to the docs about deduction and using boost::ref 3. have the static 'apply' member function return the pointer to the constructed object - just like the placement new operator 4. use file iteration instead of BOOST_PP_REPEAT for maintainability If I may, I'd like to add:
5. add in_place_factory and typed_in_place_factory for zero constructor arguments.
Quick discussion:
Often the reason against nullary function overloads is the earlier point of instantiation (nullary functions can't be templates), but that shouldn't be the problem here because 'in_place_factory's nested 'apply' is a template and 'typed_in_place_factory' is complete enough to be instantiated. For the 'inline in_place_factory0 in_place()' function the explicit 'inline' keyword is needed, however, not to break the ODR.
OK. It seems reasonable and it seems there are no unsolvable problems around, so let's add it ;-).
I remember someone mentioned this before, but I can't remember any one answered.
That probably means that there's no maintainer. If that's so, I'd pick up this role.
Well, not really. Seems Fernando Cacciola is still boosting actively (I'm CC ing this post to his hotmail address). However, I have implemented the proposed code changes (see preliminary patch attached to this message) - hoping for Fernando's OK. Regards, Tobias
...and here comes the forgotten attachment ;-| Regards, Tobias ? inplace_full.patch ? inplace_make_editable.patch Index: in_place_factory.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/utility/in_place_factory.hpp,v retrieving revision 1.6 diff -u -r1.6 in_place_factory.hpp --- in_place_factory.hpp 23 Sep 2004 17:03:54 -0000 1.6 +++ in_place_factory.hpp 4 Apr 2007 14:56:14 -0000 @@ -1,4 +1,5 @@ // Copyright (C) 2003, Fernando Luis Cacciola Carballal. +// Copyright (C) 2007, Tobias Schwinger. // // Use, modification, and distribution is subject to the Boost Software // License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -9,50 +10,78 @@ // You are welcome to contact the author at: // fernando_cacciola@hotmail.com // -#ifndef BOOST_UTILITY_INPLACE_FACTORY_25AGO2003_HPP -#define BOOST_UTILITY_INPLACE_FACTORY_25AGO2003_HPP +#ifndef BOOST_UTILITY_INPLACE_FACTORY_04APR2007_HPP +#ifndef BOOST_PP_IS_ITERATING #include <boost/utility/detail/in_place_factory_prefix.hpp> -#include <boost/type.hpp> - namespace boost { class in_place_factory_base {} ; -#define BOOST_DEFINE_INPLACE_FACTORY_CLASS(z,n,_) \ -template< BOOST_PP_ENUM_PARAMS(BOOST_PP_INC(n),class A) > \ -class BOOST_PP_CAT(in_place_factory, BOOST_PP_INC(n) ) : public in_place_factory_base \ -{ \ -public: \ -\ - BOOST_PP_CAT(in_place_factory, BOOST_PP_INC(n) ) ( BOOST_PP_ENUM_BINARY_PARAMS(BOOST_PP_INC(n),A,const& a) ) \ - : \ - BOOST_PP_ENUM( BOOST_PP_INC(n), BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT, _ ) \ - {} \ -\ - template<class T> \ - void apply ( void* address BOOST_APPEND_EXPLICIT_TEMPLATE_TYPE(T) ) const \ - { \ - new ( address ) T ( BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), m_a ) ) ; \ - } \ -\ - BOOST_PP_REPEAT( BOOST_PP_INC(n), BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL, _) \ -} ; \ -\ -template< BOOST_PP_ENUM_PARAMS(BOOST_PP_INC(n),class A) > \ -BOOST_PP_CAT(in_place_factory, BOOST_PP_INC(n) ) < BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), A ) > \ -in_place ( BOOST_PP_ENUM_BINARY_PARAMS(BOOST_PP_INC(n),A, const& a) ) \ -{ \ - return BOOST_PP_CAT(in_place_factory, BOOST_PP_INC(n) ) < BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), A ) > \ - ( BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), a ) ) ; \ -} ; \ - -BOOST_PP_REPEAT( BOOST_MAX_INPLACE_FACTORY_ARITY, BOOST_DEFINE_INPLACE_FACTORY_CLASS, BOOST_PP_EMPTY() ) +#define BOOST_PP_ITERATION_LIMITS (0, BOOST_MAX_INPLACE_FACTORY_ARITY) +#define BOOST_PP_FILENAME_1 <boost/utility/in_place_factory.hpp> +#include BOOST_PP_ITERATE() } // namespace boost #include <boost/utility/detail/in_place_factory_suffix.hpp> +#define BOOST_UTILITY_INPLACE_FACTORY_04APR2007_HPP +#else +#define N BOOST_PP_ITERATION() + +#if N +template< BOOST_PP_ENUM_PARAMS(N, class A) > +#endif +class BOOST_PP_CAT(in_place_factory,N) + : + public in_place_factory_base +{ +public: + + explicit BOOST_PP_CAT(in_place_factory,N) + ( BOOST_PP_ENUM_BINARY_PARAMS(N,A,const& a) ) +#if N > 0 + : BOOST_PP_ENUM(N, BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT, _) +#endif + {} + + template<class T> + void* apply(void* address + BOOST_APPEND_EXPLICIT_TEMPLATE_TYPE(T)) const + { + return new(address) T( BOOST_PP_ENUM_PARAMS(N, m_a) ); + } + + template<class T> + void* apply(void* address, std::size_t n + BOOST_APPEND_EXPLICIT_TEMPLATE_TYPE(T)) const + { + for(char* next = address = this->template apply<T>(address); !! --n;) + this->template apply<T>(next = next+sizeof(T)); + return address; + } + + BOOST_PP_REPEAT(N, BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL, _) +}; + +#if N > 0 +template< BOOST_PP_ENUM_PARAMS(N, class A) > +inline BOOST_PP_CAT(in_place_factory,N)< BOOST_PP_ENUM_PARAMS(N, A) > +in_place( BOOST_PP_ENUM_BINARY_PARAMS(N, A, const& a) ) +{ + return BOOST_PP_CAT(in_place_factory,N)< BOOST_PP_ENUM_PARAMS(N, A) > + ( BOOST_PP_ENUM_PARAMS(N, a) ); +} +#else +inline in_place_factory0 in_place() +{ + return in_place_factory0(); +} +#endif + +#undef N +#endif #endif Index: typed_in_place_factory.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/utility/typed_in_place_factory.hpp,v retrieving revision 1.5 diff -u -r1.5 typed_in_place_factory.hpp --- typed_in_place_factory.hpp 23 Sep 2004 17:03:54 -0000 1.5 +++ typed_in_place_factory.hpp 4 Apr 2007 14:56:14 -0000 @@ -1,4 +1,5 @@ // Copyright (C) 2003, Fernando Luis Cacciola Carballal. +// Copyright (C) 2007, Tobias Schwinger. // // Use, modification, and distribution is subject to the Boost Software // License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -9,8 +10,8 @@ // You are welcome to contact the author at: // fernando_cacciola@hotmail.com // -#ifndef BOOST_UTILITY_TYPED_INPLACE_FACTORY_25AGO2003_HPP -#define BOOST_UTILITY_TYPED_INPLACE_FACTORY_25AGO2003_HPP +#ifndef BOOST_UTILITY_TYPED_INPLACE_FACTORY_04APR2007_HPP +#ifndef BOOST_PP_IS_ITERATING #include <boost/utility/detail/in_place_factory_prefix.hpp> @@ -18,40 +19,59 @@ class typed_in_place_factory_base {} ; -#define BOOST_DEFINE_TYPED_INPLACE_FACTORY_CLASS(z,n,_) \ -template< class T, BOOST_PP_ENUM_PARAMS(BOOST_PP_INC(n),class A) > \ -class BOOST_PP_CAT(typed_in_place_factory, BOOST_PP_INC(n) ) : public typed_in_place_factory_base \ -{ \ -public: \ -\ - typedef T value_type ; \ -\ - BOOST_PP_CAT(typed_in_place_factory, BOOST_PP_INC(n) ) ( BOOST_PP_ENUM_BINARY_PARAMS(BOOST_PP_INC(n),A,const& a) ) \ - : \ - BOOST_PP_ENUM( BOOST_PP_INC(n), BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT, _ ) \ - {} \ -\ - void apply ( void* address ) const \ - { \ - new ( address ) T ( BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), m_a ) ) ; \ - } \ -\ - BOOST_PP_REPEAT( BOOST_PP_INC(n), BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL, _) \ -} ; \ -\ -template< class T, BOOST_PP_ENUM_PARAMS(BOOST_PP_INC(n),class A) > \ -BOOST_PP_CAT(typed_in_place_factory, BOOST_PP_INC(n) ) < T , BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), A ) > \ -in_place ( BOOST_PP_ENUM_BINARY_PARAMS(BOOST_PP_INC(n),A, const& a) ) \ -{ \ - return BOOST_PP_CAT(typed_in_place_factory, BOOST_PP_INC(n) ) < T, BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), A ) > \ - ( BOOST_PP_ENUM_PARAMS( BOOST_PP_INC(n), a ) ) ; \ -} ; \ - -BOOST_PP_REPEAT( BOOST_MAX_INPLACE_FACTORY_ARITY, BOOST_DEFINE_TYPED_INPLACE_FACTORY_CLASS, BOOST_PP_EMPTY() ) +#define BOOST_PP_ITERATION_LIMITS (0, BOOST_MAX_INPLACE_FACTORY_ARITY) +#define BOOST_PP_FILENAME_1 <boost/utility/typed_in_place_factory.hpp> +#include BOOST_PP_ITERATE() } // namespace boost #include <boost/utility/detail/in_place_factory_suffix.hpp> +#define BOOST_UTILITY_TYPED_INPLACE_FACTORY_04APR2007_HPP +#else +#define N BOOST_PP_ITERATION() + +template< class T BOOST_PP_ENUM_TRAILING_PARAMS(N,class A) > +class BOOST_PP_CAT(typed_in_place_factory,N) + : + public typed_in_place_factory_base +{ +public: + + typedef T value_type; + + explicit BOOST_PP_CAT(typed_in_place_factory,N) + ( BOOST_PP_ENUM_BINARY_PARAMS(N, A, const& a) ) +#if N > 0 + : BOOST_PP_ENUM(N, BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT, _) +#endif + {} + + void* apply (void* address) const + { + return new(address) T( BOOST_PP_ENUM_PARAMS(N, m_a) ); + } + + void* apply (void* address, std::size_t n) const + { + for(char* next = address = this->apply(address); !! --n;) + this->apply(next = next+sizeof(T)); + return address; + } + + BOOST_PP_REPEAT(N, BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL, _) +}; + +template< class T BOOST_PP_ENUM_TRAILING_PARAMS(N, class A) > +inline BOOST_PP_CAT(typed_in_place_factory,N)< + T BOOST_PP_ENUM_TRAILING_PARAMS(N, A) > +in_place( BOOST_PP_ENUM_BINARY_PARAMS(N, A, const& a) ) +{ + return BOOST_PP_CAT(typed_in_place_factory,N)< + T BOOST_PP_ENUM_TRAILING_PARAMS(N, A) >( BOOST_PP_ENUM_PARAMS(N, a) ); +} + +#undef N +#endif #endif Index: detail/in_place_factory_prefix.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/utility/detail/in_place_factory_prefix.hpp,v retrieving revision 1.1 diff -u -r1.1 in_place_factory_prefix.hpp --- detail/in_place_factory_prefix.hpp 21 Sep 2004 14:54:32 -0000 1.1 +++ detail/in_place_factory_prefix.hpp 4 Apr 2007 14:56:14 -0000 @@ -1,4 +1,5 @@ // Copyright (C) 2003, Fernando Luis Cacciola Carballal. +// Copyright (C) 2007, Tobias Schwinger. // // Use, modification, and distribution is subject to the Boost Software // License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -9,25 +10,27 @@ // You are welcome to contact the author at: // fernando_cacciola@hotmail.com // -#ifndef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_25AGO2003_HPP -#define BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_25AGO2003_HPP +#ifndef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_04APR2007_HPP +#define BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_04APR2007_HPP +#include <new> +#include <cstddef> #include <boost/config.hpp> +#include <boost/preprocessor/cat.hpp> +#include <boost/preprocessor/punctuation/paren.hpp> +#include <boost/preprocessor/iteration/iterate.hpp> +#include <boost/preprocessor/repetition/repeat.hpp> #include <boost/preprocessor/repetition/enum.hpp> #include <boost/preprocessor/repetition/enum_params.hpp> #include <boost/preprocessor/repetition/enum_binary_params.hpp> -#include <boost/preprocessor/cat.hpp> -#include <boost/preprocessor/arithmetic/inc.hpp> -#include <boost/preprocessor/punctuation/paren.hpp> -#include <boost/preprocessor/facilities/empty.hpp> +#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_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_ARG(z,n,_) BOOST_PP_CAT(m_a,n) #define BOOST_MAX_INPLACE_FACTORY_ARITY 10 -#undef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_25AGO2003_HPP +#undef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_04APR2007_HPP #endif Index: detail/in_place_factory_suffix.hpp =================================================================== RCS file: /cvsroot/boost/boost/boost/utility/detail/in_place_factory_suffix.hpp,v retrieving revision 1.1 diff -u -r1.1 in_place_factory_suffix.hpp --- detail/in_place_factory_suffix.hpp 21 Sep 2004 14:54:32 -0000 1.1 +++ detail/in_place_factory_suffix.hpp 4 Apr 2007 14:56:14 -0000 @@ -1,4 +1,5 @@ // Copyright (C) 2003, Fernando Luis Cacciola Carballal. +// Copyright (C) 2007, Tobias Schwinger. // // Use, modification, and distribution is subject to the Boost Software // License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -9,15 +10,14 @@ // You are welcome to contact the author at: // fernando_cacciola@hotmail.com // -#ifndef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_25AGO2003_HPP -#define BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_25AGO2003_HPP +#ifndef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_04APR2007_HPP +#define BOOST_UTILITY_DETAIL_INPLACE_FACTORY_SUFFIX_04APR2007_HPP #undef BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_INIT #undef BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_DECL -#undef BOOST_DEFINE_INPLACE_FACTORY_CLASS_MEMBER_ARG #undef BOOST_MAX_INPLACE_FACTORY_ARITY -#undef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_25AGO2003_HPP +#undef BOOST_UTILITY_DETAIL_INPLACE_FACTORY_PREFIX_04APR2007_HPP #endif
Hi Tobias,
...and here comes the forgotten attachment ;-|
I haven't looked at it already but off the top of my head your suggestions are welcome (for 1.35 of course). In fact, I fixed the zero-argument problem but couldn't make it time for 1.34, so it sits on my local working copy of HEAD. Missing 1.34 is a pity, but with your improvements, I can at least argue that the new version will have more than just zero-arg overloads. Best Fernando Cacciola
Hello Tobias, Wednesday, April 4, 2007, 1:28:31 PM, you wrote: [snip]
If I may, I'd like to add:
5. add in_place_factory and typed_in_place_factory for zero constructor arguments.
Quick discussion:
Often the reason against nullary function overloads is the earlier point of instantiation (nullary functions can't be templates), but that shouldn't be the problem here because 'in_place_factory's nested 'apply' is a template and 'typed_in_place_factory' is complete enough to be instantiated. For the 'inline in_place_factory0 in_place()' function the explicit 'inline' keyword is needed, however, not to break the ODR.
OK. It seems reasonable and it seems there are no unsolvable problems around, so let's add it ;-).
I remember someone mentioned this before, but I can't remember any one answered.
That probably means that there's no maintainer. If that's so, I'd pick up this role.
Thanks a lot, Tobias! :) -- Best regards, Andrey mailto:andysem@mail.ru
participants (3)
-
Andrey Semashev -
Fernando Cacciola -
Tobias Schwinger