Re: [boost] Improving the assignment operators of various Boost types

Subject: Re: [boost] Improving the assignment operators of various Boost types
From: David Abrahams (dave_at_[hidden])
Date: 2008-09-09 19:35:19

on Tue Sep 09 2008, Niels Dekker - mail address until 2008-12-31 <nd_mail_address_valid_until_2008-12-31-AT-xs4all.nl> wrote:

> David Abrahams wrote:
>> In the case of copy-elision for by-value arguments and return values,
>> the compiler is explicitly allowed to _assume_ there is no semantic
>> difference between the original rvalue and its copy.  That's low-hanging
>> fruit for a compiler writer, that pays huge dividends.
>
> So far I've found 7 copy assignment operators that could be improved by
> having their argument "by value", instead of "by const reference", at
> the following locations, in trunk/boost:
>
>  - any.hpp(64): boost::any
>  - function\function_template.hpp(916): boost::function
>  - intrusive_ptr.hpp(114): intrusive_ptr
>  - multi_index_container.hpp(269): multi_index_container
>  - interprocess\allocators\adaptive_pool.hpp(136): adaptive_pool_base
>  - spirit\home\classic\tree\common.hpp(101): spirit::tree_node
>  - spirit\home\classic\tree\common.hpp(624): spirit::tree_match
>
> Am I correct, or am I possibly missing some others?  

Well, I just took a look through boost/iterator/iterator_facade.hpp and
found several that could be "conditionally improved" in the case where
the rhs has a swap (you probably remember that we discussed detecting that in
http://news.gmane.org/find-root.php?message_id=%3c16708896C72A4A51BB534B1187D29689%404lkebivusnd%3e).

I think that also goes for many compiler-generated assignment operators,
so arguably your search should include files with no explicit operator=.
I suppose this optimization does us no good in base classes, so things
like iterator_facade would be exempt, but the specialized adaptors could
still benefit from it.

> I wouldn't mind
> creating some tickets (one per source file), requesting to have their
> argument "by value", as we discussed at comp.lang.c++.moderated, "Re: Is
> self assignment test valid?".

Might I suggest one per library?

> Basically it was concluded that /if/ an assignment operator is
> implemented by means of copy-and-swap, it should preferably be done as
> follows: 
>
>   T& operator=(T arg)
>   {
>     arg.swap(*this);
>     return *this;
>   }

In generic code I strongly prefer

   T& operator=(T arg)
   {
     swap(*this, arg);
     return *this;
   }

since swap members have little generic value and therefore are only ever
implemented as a convenience.

-- 
Dave Abrahams
BoostPro Computing
http://www.boostpro.com