Subject: [Boost-commit] svn:boost r66369 - sandbox/function/boost/function
From: dsaritz_at_[hidden]
Date: 2010-11-03 05:51:30
Author: psiha
Date: 2010-11-03 05:51:25 EDT (Wed, 03 Nov 2010)
New Revision: 66369
URL: http://svn.boost.org/trac/boost/changeset/66369
Log:
Fixed bugs/leaks introduced in the last commit.
Text files modified:
sandbox/function/boost/function/function_base.hpp | 47 +++++++------------------------
sandbox/function/boost/function/function_template.hpp | 58 +++++++++++++++++++++++++++++++++------
2 files changed, 59 insertions(+), 46 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-03 05:51:25 EDT (Wed, 03 Nov 2010)
@@ -1158,38 +1158,12 @@
}
// 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
+ // See the note for the no_eh_state_construction_trick() helper in
+ // function_template.hpp to see the purpose of this constructor.
+ // (03.11.2010.) (Domagoj Saric)
+ function_base( detail::function::vtable const & vtable ) { BF_ASSUME( &vtable == p_vtable_ ); }
+
+ ~function_base() { destroy(); }
template <class EmptyHandler>
void swap( function_base & other, detail::function::vtable const & empty_handler_vtable );
@@ -1358,7 +1332,7 @@
this->p_vtable_ = &functor_vtable;
}
- template<typename EmptyHandler, typename FunctionObj, typename Allocator>
+ template <typename EmptyHandler, typename FunctionObj, typename Allocator>
void actual_assign
(
FunctionObj const & f,
@@ -1378,7 +1352,7 @@
this->swap<EmptyHandler>( tmp, empty_handler_vtable );
}
-protected:
+private:
void destroy() { get_vtable().destroy( this->functor_ ); }
private:
@@ -1759,8 +1733,9 @@
if ( direct )
{
// Implementation note:
- // See the note for the function_base destructor as to why a null
- // vtable is allowed here.
+ // See the note for the no_eh_state_construction_trick() helper in
+ // function_template.hpp as to why a null vtable is allowed and expected
+ // here.
// (02.11.2010.) (Domagoj Saric)
BOOST_ASSERT( this->p_vtable_ == NULL );
typedef typename get_functor_manager<FunctionObj, Allocator>::type functor_manager;
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-03 05:51:25 EDT (Wed, 03 Nov 2010)
@@ -256,7 +256,6 @@
#endif // BOOST_MSVC
}
- ~BOOST_FUNCTION_FUNCTION() { function_base::destroy(); }
// MSVC chokes if the following two constructors are collapsed into
// one with a default parameter.
@@ -271,9 +270,8 @@
int>::type = 0
#endif // BOOST_NO_SFINAE
)
- {
- this->do_assign<true, Functor>( f );
- }
+ : function_base( no_eh_state_construction_trick( f ) )
+ {}
template <typename Functor, typename Allocator>
BOOST_FUNCTION_FUNCTION
@@ -287,9 +285,8 @@
int>::type = 0
#endif // BOOST_NO_SFINAE
)
- {
- this->do_assign<true, Functor>( f, a );
- }
+ : function_base( no_eh_state_construction_trick( f, a ) )
+ {}
#ifndef BOOST_NO_SFINAE
@@ -395,13 +392,13 @@
#ifndef BOOST_NO_SFINAE
- BOOST_FUNCTION_FUNCTION& operator=(clear_type*)
+ BOOST_FUNCTION_FUNCTION & operator=(clear_type*)
{
this->clear();
return *this;
}
#else
- BOOST_FUNCTION_FUNCTION& operator=(int zero)
+ BOOST_FUNCTION_FUNCTION & operator=(int zero)
{
BOOST_ASSERT(zero == 0);
this->clear();
@@ -633,7 +630,7 @@
void dispatch_assign( FunctionObj const & f, Allocator const a, detail::function::function_obj_ref_tag ) { do_assign<direct, typename FunctionObj::type>( f.get(), f , a ); }
template <bool direct, typename ActualFunctor, typename StoredFunctor, typename ActualFunctorAllocator>
- void do_assign( ActualFunctor const & original_functor, StoredFunctor const & stored_functor, ActualFunctorAllocator const a )
+ void do_assign( ActualFunctor const &, StoredFunctor const & stored_functor, ActualFunctorAllocator const a )
{
if
(
@@ -677,6 +674,47 @@
);
}
}
+
+ // Implementation note:
+ // Simply default-constructing funciton_base and then performing proper
+ // initialization in the body of the derived class constructor has
+ // unfortunate efficiency implications because it creates unnecessary
+ // EH states (=unnecessary bloat) in case of non-trivial
+ // (i.e. fallible/throwable) constructors of derived classes (when
+ // constructing from/with complex function objects).
+ // In such cases the compiler has to generate EH code to call the
+ // (non-trivial) 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, the helper functions below are used as a
+ // quick-hack to actually construct the function_base/
+ // BOOST_FUNCTION_FUNCTION object before the function_base constructor is
+ // called. The entire object is also cleared beforehand in debugging builds
+ // to allow checking that the vtable and/or the function_buffer are not used
+ // before being initialized.
+ // (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
+ /// there).
+ /// (02.11.2010.) (Domagoj Saric)
+ template <typename FunctionObj, typename Allocator>
+ detail::function::vtable const & no_eh_state_construction_trick( FunctionObj const & f, Allocator const a )
+ {
+ detail::function::debug_clear( *this );
+ do_assign<true>( f, a );
+ return function_base::get_vtable();
+ }
+
+ template <typename FunctionObj>
+ detail::function::vtable const & no_eh_state_construction_trick( FunctionObj const & f )
+ {
+ detail::function::debug_clear( *this );
+ do_assign<true>( f, detail::function::fallocator<FunctionObj>() );
+ return function_base::get_vtable();
+ }
};
template<typename R BOOST_FUNCTION_COMMA BOOST_FUNCTION_TEMPLATE_PARMS, class PolicyList>