[Boost-commit] svn:boost r66369 - sandbox/function/boost/function

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>