[Boost-bugs] [Boost C++ Libraries] #1910: function::swap should avoid doing memory allocations

Subject: [Boost-bugs] [Boost C++ Libraries] #1910: function::swap should avoid doing memory allocations
From: Boost C++ Libraries (noreply_at_[hidden])
Date: 2008-05-10 13:01:48

#1910: function::swap should avoid doing memory allocations
--------------------------+-------------------------------------------------
 Reporter:  niels_dekker  |       Owner:  dgregor 
     Type:  Bugs          |      Status:  new     
Milestone:  Boost 1.36.0  |   Component:  function
  Version:  Boost 1.35.0  |    Severity:  Problem 
 Keywords:                |  
--------------------------+-------------------------------------------------
 This ticket was triggered by a posting from Arvid Norberg,
 [http://lists.boost.org/Archives/boost/2008/02/133534.php

 boost::function<>::swap() can throw]. He was surprised to see that
 {{{boost::function::swap}}} can throw an exception, in case of a memory
 allocation failure. And he also remarked that TR1's {{{function::swap}}}
 is specified ''not'' to throw.

 The current {{{function::swap}}}
 ([http://svn.boost.org/trac/boost/browser/trunk/boost/function/function_template.hpp?rev=43884

 function_template.hpp, from trunk revision 43884]) is implemented by doing
 three copy operations:
 {{{
     void swap(BOOST_FUNCTION_FUNCTION& other)
     {
       if (&other == this)
         return;

       BOOST_FUNCTION_FUNCTION tmp = *this;
       *this = other;
       other = tmp;
     }
 }}}

 I think that a major cause of exceptions can be taken away, by having
 {{{function::swap}}} implemented by means of ''moving'', instead of
 copying:

 {{{
     void swap(BOOST_FUNCTION_FUNCTION& other)
     {
       if (&other == this)
         return;

       BOOST_FUNCTION_FUNCTION tmp;
       tmp.move_assign(*this);
       this->move_assign(other);
       other.move_assign(tmp);
     }
 }}}

 This new {{{boost::function::move_assign}}} member function would in most
 cases just copy the function object held by its argument to {{{*this}}},
 just like {{{boost::function}}}'s copy assignment operator does. But if
 the argument would hold its function object within a buffer ''allocated on
 the heap'', {{{move_assign}}} would pass the buffer to {{{*this}}}, and
 set the argument's buffer pointer to {{{NULL}}}.

 The patch has {{{boost::function::move_assign}}} added to
 {{{function_template.hpp}}}.
 Out of modesty, I declared {{{move_assign}}} as a ''private'' member
 function, but feel free to make it public. Moreover, it could be changed
 to move ''from'' {{{*this}}} ''to'' its argument (and renamed, e.g., to
 {{{move_to}}}), so that it would support moving a temporary ''(rvalue)''.
 But I think that's beyond the scope of this ticket...

 == Implementation details ==

 The attached patch has a boolean argument added to all {{{manage}}}
 functions, telling whether or not the buffer should be moved.  Thereby,
 the implementation of {{{move_assign}}} is quite similar to the existing
 {{{assign_to_own}}}, having the same call to {{{f.vtable->manager}}}, but
 passing {{{true}}} as last argument.

 This boolean argument {{{move_buffer}}} is only relevant to the specific
 {{{manager}}} overload for function objects that require heap allocation,
 which does in the patched version of {{{function_base.hpp}}}:
 {{{
   if (move_buffer)
   {
     out_buffer.obj_ptr = in_buffer.obj_ptr;
     in_buffer.obj_ptr = 0;
   }
 }}}

 I made {{{obj_ptr}}} ''mutable'', to work around the fact that
 {{{in_buffer}}} is a ''const'' reference. Instead, I guess a
 {{{const_cast}}} could be used, but I didn't have the guts to do so.  ;-)

-- 
Ticket URL: <http://svn.boost.org/trac/boost/ticket/1910>
Boost C++ Libraries <http://www.boost.org/>
Boost provides free peer-reviewed portable C++ source libraries.