Subject: [Boost-commit] svn:boost r66360 - sandbox/function/boost/function
From: dsaritz_at_[hidden]
Date: 2010-11-02 15:45:10
Author: psiha
Date: 2010-11-02 15:45:08 EDT (Tue, 02 Nov 2010)
New Revision: 66360
URL: http://svn.boost.org/trac/boost/changeset/66360
Log:
Refactored (copy) construction and destruction code to avoid creating unnecessary EH states (and related bloat).
Properly encapsulated function_base data members.
Minor other refactoring and stylistic changes.
Text files modified:
sandbox/function/boost/function/function_base.hpp | 70 ++++++++++++++++++++++++++++++++--------
sandbox/function/boost/function/function_template.hpp | 30 ++++++----------
2 files changed, 67 insertions(+), 33 deletions(-)
Modified: sandbox/function/boost/function/function_base.hpp
==============================================================================
--- sandbox/function/boost/function/function_base.hpp (original)
+++ sandbox/function/boost/function/function_base.hpp 2010-11-02 15:45:08 EDT (Tue, 02 Nov 2010)
@@ -1148,14 +1148,54 @@
template <class EmptyHandler>
class safe_mover;
-public:
+protected:
+ function_base() { detail::function::debug_clear( *this ); }
+ function_base( function_base const & other )
+ {
+ detail::function::debug_clear( *this );
+ assign_boost_function_direct( other );
+ }
+
template <class EmptyHandler>
function_base( detail::function::vtable const & empty_handler_vtable, EmptyHandler )
{
detail::function::debug_clear( *this );
this->clear<true, EmptyHandler>( empty_handler_vtable );
}
- ~function_base() { destroy(); }
+
+ // Implementation note:
+ // Ideally destruction/cleanup could be simply placed into the
+ // function_base destructor. However, this has unfortunate efficiency
+ // implications because it creates unnecessary EH states (=unnecessary code)
+ // in non-trivial (i.e. fallible/throwable) constructors of derived classes.
+ // In such cases the compiler has to generate EH code to call the
+ // function_base destructor if the derived-class constructor fails after
+ // function_base is already constructed. This is completely redundant
+ // because initially function_base is/was always initialized with the empty
+ // handler for which no destruction is necessary but the compiler does not
+ // see this because of the indirect vtable call.
+ // Because of the above issue, default constructed function_base objects
+ // with a null/uninitialized vtable pointer are allowed and the duty to
+ // either throw an exception or properly initialized the vtable (along with
+ // the entire object) is shifted to the derived class destructor.
+ // (02.11.2010.) (Domagoj Saric)
+ // Implementation note:
+ // A by-the-book protected (empty) destructor is still added to prevent
+ // accidental deletion of a function_base pointer. However, because even a
+ // trivial empty destructor creates EH states with MSVC++ (even version 10)
+ // and possibly other compilers, it is removed from release builds.
+ // (02.11.2010.) (Domagoj Saric)
+ /// \todo Devise a cleaner way to deal with all of this (maybe move/add more
+ /// template methods to function_base so that it can call assign methods
+ /// from its template constructors thereby moving all construction code
+ /// here).
+ /// (02.11.2010.) (Domagoj Saric)
+#ifdef _DEBUG
+ ~function_base()
+ {
+ //...destroy();
+ }
+#endif // _DEBUG
template <class EmptyHandler>
void swap( function_base & other, detail::function::vtable const & empty_handler_vtable );
@@ -1227,6 +1267,11 @@
return *p_vtable_;
}
+ detail::function::function_buffer & functor() const
+ {
+ return functor_;
+ }
+
template <bool direct, class EmptyHandler>
BF_NOTHROW
void clear( detail::function::vtable const & empty_handler_vtable )
@@ -1248,8 +1293,8 @@
private: // Assignment from another boost function helpers.
void assign_boost_function_direct( function_base const & source )
{
- source.p_vtable_->clone( source.functor_, this->functor_ );
- p_vtable_ = source.p_vtable_;
+ source.get_vtable().clone( source.functor_, this->functor_ );
+ p_vtable_ = &source.get_vtable();
}
template <class EmptyHandler>
@@ -1335,10 +1380,10 @@
this->swap<EmptyHandler>( tmp, empty_handler_vtable );
}
-private:
+protected:
void destroy() { get_vtable().destroy( this->functor_ ); }
-protected:
+private:
// Fix/properly encapsulate these members and use the function_buffer_holder.
detail::function::vtable const * p_vtable_;
mutable detail::function::function_buffer functor_ ;
@@ -1703,14 +1748,11 @@
else
if ( direct )
{
- BOOST_ASSERT
- (
- ( this->p_vtable_ == &empty_handler_vtable ) ||
- (
- is_same<EmptyHandler, FunctionObj>::value &&
- this->p_vtable_ == NULL
- )
- );
+ // Implementation note:
+ // See the note for the function_base destructor as to why a null
+ // vtable is allowed here.
+ // (02.11.2010.) (Domagoj Saric)
+ BOOST_ASSERT( this->p_vtable_ == NULL );
typedef typename get_functor_manager<FunctionObj, Allocator>::type functor_manager;
functor_manager::assign( f, this->functor_, a );
this->p_vtable_ = &functor_vtable;
Modified: sandbox/function/boost/function/function_template.hpp
==============================================================================
--- sandbox/function/boost/function/function_template.hpp (original)
+++ sandbox/function/boost/function/function_template.hpp 2010-11-02 15:45:08 EDT (Tue, 02 Nov 2010)
@@ -150,24 +150,19 @@
= default_policies
#endif // BOOST_FUNCTION_MAX_ARGS > 10
>
- class BOOST_FUNCTION_FUNCTION : public function_base
-
+ class BOOST_FUNCTION_FUNCTION
+ : public function_base
#if BOOST_FUNCTION_NUM_ARGS == 1
-
, public std::unary_function<T0,R>
-
#elif BOOST_FUNCTION_NUM_ARGS == 2
-
, public std::binary_function<T0,T1,R>
-
#endif
-
{
private: // Actual policies deduction section.
//mpl::at<AssocSeq,Key,Default> does not yet exist so...:
- typedef throw_on_empty default_empty_handler ;
- typedef mpl::false_ default_nothrow_policy;
+ typedef throw_on_empty default_empty_handler ;
+ typedef mpl::false_ default_nothrow_policy;
typedef typename mpl::at<PolicyList, empty_handler_t>::type user_specified_empty_handler ;
typedef typename mpl::at<PolicyList, is_no_throw_t >::type user_specified_nothrow_policy;
@@ -261,6 +256,8 @@
#endif // BOOST_MSVC
}
+ ~BOOST_FUNCTION_FUNCTION() { function_base::destroy(); }
+
// MSVC chokes if the following two constructors are collapsed into
// one with a default parameter.
template <typename Functor>
@@ -274,8 +271,6 @@
int>::type = 0
#endif // BOOST_NO_SFINAE
)
- :
- function_base( empty_handler_vtable(), base_empty_handler() )
{
this->do_assign<true, Functor>( f );
}
@@ -292,8 +287,6 @@
int>::type = 0
#endif // BOOST_NO_SFINAE
)
- :
- function_base( empty_handler_vtable(), base_empty_handler() )
{
this->do_assign<true, Functor>( f, a );
}
@@ -308,10 +301,9 @@
}
#endif
- BOOST_FUNCTION_FUNCTION(const BOOST_FUNCTION_FUNCTION& f) : function_base( empty_handler_vtable(), base_empty_handler() )
- {
- this->do_assign<true>( f );
- }
+ BOOST_FUNCTION_FUNCTION( BOOST_FUNCTION_FUNCTION const & f )
+ : function_base( static_cast<function_base const &>( f ) )
+ {}
/// Determine if the function is empty (i.e., has empty target).
bool empty() const { return p_vtable_->is_empty_handler_vtable(); }
@@ -517,14 +509,14 @@
result_type do_invoke( BOOST_FUNCTION_PARMS BOOST_FUNCTION_COMMA mpl::true_ /*this call*/ ) const
{
typedef result_type (detail::function::function_buffer::* invoker_type)(BOOST_FUNCTION_TEMPLATE_ARGS);
- return (functor_.*(get_vtable(). BOOST_NESTED_TEMPLATE invoker<invoker_type>()))(BOOST_FUNCTION_ARGS);
+ return (functor().*(get_vtable(). BOOST_NESTED_TEMPLATE invoker<invoker_type>()))(BOOST_FUNCTION_ARGS);
}
BF_FORCEINLINE
result_type do_invoke( BOOST_FUNCTION_PARMS BOOST_FUNCTION_COMMA mpl::false_ /*free call*/ ) const
{
typedef result_type (* invoker_type)( BOOST_FUNCTION_TEMPLATE_ARGS BOOST_FUNCTION_COMMA detail::function::function_buffer & );
- return get_vtable(). BOOST_NESTED_TEMPLATE invoker<invoker_type>()( BOOST_FUNCTION_ARGS BOOST_FUNCTION_COMMA functor_ );
+ return get_vtable(). BOOST_NESTED_TEMPLATE invoker<invoker_type>()( BOOST_FUNCTION_ARGS BOOST_FUNCTION_COMMA functor() );
}