<div class="gmail_quote">On Tue, May 29, 2012 at 6:17 AM, Benjamin Kramer <span dir="ltr"><<a href="mailto:benny.kra@googlemail.com" target="_blank">benny.kra@googlemail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
On 29.05.2012, at 01:28, Richard Smith wrote:<br>
<br>
> On Sun, May 27, 2012 at 1:46 PM, Benjamin Kramer <<a href="mailto:benny.kra@googlemail.com">benny.kra@googlemail.com</a>> wrote:<br>
> Author: d0k<br>
> Date: Sun May 27 15:46:04 2012<br>
> New Revision: 157546<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=157546&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=157546&view=rev</a><br>
> Log:<br>
> IntrusiveRefCntPtr: Use the same pattern as the other operator= overloads when using rvalue refs.<br>
><br>
> Modified:<br>
>    llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h<br>
><br>
> Modified: llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h?rev=157546&r1=157545&r2=157546&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h?rev=157546&r1=157545&r2=157546&view=diff</a><br>

> ==============================================================================<br>
> --- llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h (original)<br>
> +++ llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h Sun May 27 15:46:04 2012<br>
> @@ -23,6 +23,7 @@<br>
><br>
>  #include "llvm/Support/Casting.h"<br>
>  #include "llvm/Support/Compiler.h"<br>
> +#include <memory><br>
><br>
>  namespace llvm {<br>
><br>
> @@ -146,15 +147,13 @@<br>
><br>
>  #if LLVM_USE_RVALUE_REFERENCES<br>
>     IntrusiveRefCntPtr& operator=(IntrusiveRefCntPtr&& S) {<br>
> -      Obj = S.Obj;<br>
> -      S.Obj = 0;<br>
> +      this_type(std::move(S)).swap(*this);<br>
>       return *this;<br>
>     }<br>
><br>
>     template <class X><br>
>     IntrusiveRefCntPtr& operator=(IntrusiveRefCntPtr<X>&& S) {<br>
> -      Obj = S.getPtr();<br>
> -      S.Obj = 0;<br>
> +      this_type(std::move(S)).swap(*this);<br>
>       return *this;<br>
>     }<br>
><br>
> How about replacing all five operator= overloads (duplicating the work of the constructors) and the 'replace' member function with just:<br>
><br>
> IntrusiveRefCntPtr& operator=(IntrusiveRefCntPtr O) {<br>
>   swap(O);<br>
>   return *this;<br>
> }<br>
><br>
> As well as simplifying the code, this would give us move semantics in some cases in C++98.<br>
<br>
</div></div>The current design of operator= is basically copied from c++11's std::shared_ptr, my c++-fu is too weak to prove that this fully is equivalent. The whole point of the moving RefCntPtrs is avoiding unnecessary retain/releases because it requires dereferencing the pointer which can be expensive. Doesn't your solution add an extra retain/release in the move case?</blockquote>
<div><br></div><div>No, it performs the same constructions and destructions. Your formulation move-constructs and destroys an IntrusiveRefCntPtr object in the "<span style>this_type(std::move(S))" expression, whereas mine does the same when constructing the "O" parameter.</span></div>
<div><br></div><div>[My approach has a tiny advantage if the construction of "O" can be elided: in that case, it's one move-constructor call and one no-op destructor call cheaper, but both of those calls should be trivial to optimize away, so I don't think that's significant.]</div>
</div>