Adds move assignment operator to scoped_refptr

Adds move assignment operator to scoped_refptr
Implementation borrowed from chromium.

BUG=webrtc:5556
R=pthatcher@webrtc.org

Committed: https://chromium.googlesource.com/external/webrtc/+/70cfc14af67ce2f85143fefa9ff28703445d0f43

[danilchap, 2016-07-28 16:32:52]

danilchap@webrtc.org changed reviewers:
+ deadbeef@webrtc.org, pthatcher@webrtc.org

[danilchap, 2016-07-28 16:32:53]

move constructor is not enough, move operator= should be too.

[pthatcher1, 2016-07-29 16:39:24]

lgtm

[danilchap, 2016-07-29 17:15:15]

Description was changed from

==========
Adds move assignment operator to scoped_refptr
Implementation borrowed from chromium.

BUG=webrtc:5556
==========

to

==========
Adds move assignment operator to scoped_refptr
Implementation borrowed from chromium.

BUG=webrtc:5556
R=pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/70cfc14af67ce2f85143fefa9...
==========

[commit-bot: I haz the power, 2016-07-29 17:15:16]

Description was changed from

==========
Adds move assignment operator to scoped_refptr
Implementation borrowed from chromium.

BUG=webrtc:5556
R=pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/70cfc14af67ce2f85143fefa9...
==========

to

==========
Adds move assignment operator to scoped_refptr
Implementation borrowed from chromium.

BUG=webrtc:5556
R=pthatcher@webrtc.org

Committed: https://crrev.com/70cfc14af67ce2f85143fefa9ff28703445d0f43
Cr-Commit-Position: refs/heads/master@{#13577}
==========

[commit-bot: I haz the power, 2016-07-29 17:15:16]

Patchset 1 (id:??) landed as
https://crrev.com/70cfc14af67ce2f85143fefa9ff28703445d0f43
Cr-Commit-Position: refs/heads/master@{#13577}

[danilchap, 2016-07-29 17:15:17]

Committed patchset #1 (id:1) manually as
70cfc14af67ce2f85143fefa9ff28703445d0f43 (presubmit successful).

[Taylor Brandstetter, 2016-08-01 20:00:36]

https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h
File webrtc/base/scoped_ref_ptr.h (right):

https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h#...
webrtc/base/scoped_ref_ptr.h:138: scoped_refptr<T>(std::move(r)).swap(*this);
I realize this uses the same method as Chromium's scoped_refptr, but do you know
why the std::move is used? Isn't `r` already movable, and so isn't this
std::move a no-op?

[danilchap, 2016-08-02 09:24:06]

On 2016/08/01 20:00:36, Taylor Brandstetter wrote:
> https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h
> File webrtc/base/scoped_ref_ptr.h (right):
> 
>
https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h#...
> webrtc/base/scoped_ref_ptr.h:138: scoped_refptr<T>(std::move(r)).swap(*this);
> I realize this uses the same method as Chromium's scoped_refptr, but do you
know
> why the std::move is used? Isn't `r` already movable, and so isn't this
> std::move a no-op?

tl;dr without std::move copy constructor will be called to create temporary
object

Yes and no.
Yes r is movable because scoped_ref_ptr has movable constructor,
yes std::move is a no-op because it is always a no-op (it is just a cast from T
to T&&)

No, 'r' is not movable. r parameter temporary means that at the end of the
function r can end up in any valid state, but that doesn't mean it can be
implicitly moved from in the middle of the function.
No, std::move is not a no-op because it turns r from lvalue to xvalue. (See 1st
rules for lvalue and xvalue at
http://en.cppreference.com/w/cpp/language/value_category)

e.g. imagine this valid code:
scoped_refptr<T>& operator=(scoped_refptr<T>&& r) {
  scoped_refptr<T>(r).swap(*this);
  r->DoSomethingElse();
}

Here r is used after it was used create a temporary object, so can't be allowed
to be moved from in the 1st line.
So 1st line need a hint when object will not be used later in the body of the
function. That what std::move() does.

[Taylor Brandstetter, 2016-08-02 17:18:25]

On 2016/08/02 09:24:06, danilchap wrote:
> On 2016/08/01 20:00:36, Taylor Brandstetter wrote:
> > https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h
> > File webrtc/base/scoped_ref_ptr.h (right):
> > 
> >
>
https://codereview.webrtc.org/2193443002/diff/1/webrtc/base/scoped_ref_ptr.h#...
> > webrtc/base/scoped_ref_ptr.h:138:
scoped_refptr<T>(std::move(r)).swap(*this);
> > I realize this uses the same method as Chromium's scoped_refptr, but do you
> know
> > why the std::move is used? Isn't `r` already movable, and so isn't this
> > std::move a no-op?
> 
> tl;dr without std::move copy constructor will be called to create temporary
> object
> 
> Yes and no.
> Yes r is movable because scoped_ref_ptr has movable constructor,
> yes std::move is a no-op because it is always a no-op (it is just a cast from
T
> to T&&)
> 
> No, 'r' is not movable. r parameter temporary means that at the end of the
> function r can end up in any valid state, but that doesn't mean it can be
> implicitly moved from in the middle of the function.
> No, std::move is not a no-op because it turns r from lvalue to xvalue. (See
1st
> rules for lvalue and xvalue at
> http://en.cppreference.com/w/cpp/language/value_category)
> 
> e.g. imagine this valid code:
> scoped_refptr<T>& operator=(scoped_refptr<T>&& r) {
>   scoped_refptr<T>(r).swap(*this);
>   r->DoSomethingElse();
> }
> 
> Here r is used after it was used create a temporary object, so can't be
allowed
> to be moved from in the 1st line.
> So 1st line need a hint when object will not be used later in the body of the
> function. That what std::move() does.

It's amazing I can use C++ for years and still not completely understand these
things. So a named rvalue reference is actually an lvalue; that's what I was
missing. Thanks for explaining.