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

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() );
     }