id,summary,reporter,owner,description,type,status,milestone,component,version,severity,resolution,keywords,cc
1910,function::swap should avoid doing memory allocations,niels_dekker,Douglas Gregor,"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.  ;-)",Bugs,closed,Boost 1.36.0,function,Boost 1.35.0,Problem,fixed,,nd_mail_address_valid_until_2008-12-31@…