Replace use of .Pass() with crashpad::move().

Replace use of .Pass() with crashpad::move().

Since C++11 library support isn't available everywhere crashpad is
compiled, add our own move() method in the crashpad namespace to replace
std::move() for now. Replace uses of .Pass() with this method.

R=mark@chromium.org, scottmg@chromium.org
BUG=chromium:557422
Committed: https://chromium.googlesource.com/crashpad/crashpad/+/6bebb10829332dee5c7315abafb0a8bf32840c15

[scottmg, 2015-11-30 21:02:29]

"trybot": https://gist.github.com/sgraham/48bba94af9aac8b38b0e Need to change
mini_chromium too? :(

[danakj, 2015-11-30 21:07:10]

Oops, my signature on move() was a bit wrong. Fixed, and fixed 80cols issues.

[danakj, 2015-11-30 21:08:26]

On 2015/11/30 21:02:29, scottmg wrote:
> "trybot": https://gist.github.com/sgraham/48bba94af9aac8b38b0e Need to change
> mini_chromium too? :(

ack... yes it's behind and still using old emulation in ScopedGeneric.. D:

[danakj, 2015-11-30 21:23:11]

On 2015/11/30 21:08:26, danakj (behind on reviews) wrote:
> On 2015/11/30 21:02:29, scottmg wrote:
> > "trybot": https://gist.github.com/sgraham/48bba94af9aac8b38b0e Need to
change
> > mini_chromium too? :(
> 
> ack... yes it's behind and still using old emulation in ScopedGeneric.. D:

mini_chromium patch #1: https://github.com/mark-chromium/mini_chromium/pull/2

With that, this should build.. in theory.

[scottmg, 2015-11-30 21:25:14]

On 2015/11/30 21:23:11, danakj (behind on reviews) wrote:
> On 2015/11/30 21:08:26, danakj (behind on reviews) wrote:
> > On 2015/11/30 21:02:29, scottmg wrote:
> > > "trybot": https://gist.github.com/sgraham/48bba94af9aac8b38b0e Need to
> change
> > > mini_chromium too? :(
> > 
> > ack... yes it's behind and still using old emulation in ScopedGeneric.. D:
> 
> mini_chromium patch #1: https://github.com/mark-chromium/mini_chromium/pull/2
> 
> With that, this should build.. in theory.

I'm not sure what that repo is, but the mini_chromium that we're pulling here is
https://chromium.googlesource.com/chromium/mini_chromium/ and uses
codereview.chromium.org for patches.

[Mark Mentovai, 2015-11-30 21:26:29]

That’s an old mirror of the real one.

[danakj, 2015-11-30 21:38:31]

On 2015/11/30 21:26:29, Mark Mentovai wrote:
> That’s an old mirror of the real one.

Ah ok. So the real mini-chromium has rvalue refs for scoped_ptr and
ScopedGeneric already. PHEW.

So, I think this should work.. mind trying the latest patch set?

[scottmg, 2015-11-30 21:40:41]

On 2015/11/30 21:38:31, danakj (behind on reviews) wrote:
> On 2015/11/30 21:26:29, Mark Mentovai wrote:
> > That’s an old mirror of the real one.
> 
> Ah ok. So the real mini-chromium has rvalue refs for scoped_ptr and
> ScopedGeneric already. PHEW.
> 
> So, I think this should work.. mind trying the latest patch set?

Sorry for the misdirection. Yes, ps#4 compiles fine on Windows and Mac.

[scottmg, 2015-11-30 21:45:35]

Do they need to be crashpad::move(), rather than just move()? I guess the idea
is that it's easier to switch to std::move later?

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h
File util/misc/move.h (right):

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode20
util/misc/move.h:20: // A replacement for std::remove_reference until C++11
library support is
These need to be //! \brief, and indent the second line + 4. Same for comment on
remove_reference.

[Mark Mentovai, 2015-11-30 21:47:51]

https://codereview.chromium.org/1483073004/diff/60001/client/crashpad_client_...
File client/crashpad_client_mac.cc (right):

https://codereview.chromium.org/1483073004/diff/60001/client/crashpad_client_...
client/crashpad_client_mac.cc:162: crashpad::move(receive_right),
The extra crashpad:: qualification when you’re already in namespace crashpad is
probably going to trip up clang in some warning configuration that I thought we
did use. But I’ll let it slide if you didn’t hit real warnings because it makes
the uses of crashpad::move easier to grep for.

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h
File util/misc/move.h (right):

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode15
util/misc/move.h:15: #ifndef CRASHPAD_UTIL_MISC_MOVE_H_
Ought to be in util/stdlib, where we put stdlibby things.

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode27
util/misc/move.h:27: // A replacement for std::move() until C++11 library
support is available.
Does it work to define crashpad::move() and these other things in terms of
std::move() if CXX_LIBRARY_VERSION >= 2011? Get CXX_LIBRARY_VERSION from
"util/stdlib/cxx.h".

[danakj, 2015-11-30 22:06:54]

On 2015/11/30 21:45:35, scottmg wrote:
> Do they need to be crashpad::move(), rather than just move()? I guess the idea
> is that it's easier to switch to std::move later?

Ya, it's not needed but it avoids ambiguity if someone did using namespace std
or using std::move or something. It's obvious this way (I don't like naming
things to collide with std::).

> https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h
> File util/misc/move.h (right):
> 
>
https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode20
> util/misc/move.h:20: // A replacement for std::remove_reference until C++11
> library support is
> These need to be //! \brief, and indent the second line + 4. Same for comment
on
> remove_reference.

[danakj, 2015-11-30 22:07:02]

PTAL

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h
File util/misc/move.h (right):

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode15
util/misc/move.h:15: #ifndef CRASHPAD_UTIL_MISC_MOVE_H_
On 2015/11/30 21:47:51, Mark Mentovai wrote:
> Ought to be in util/stdlib, where we put stdlibby things.

Done.

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode20
util/misc/move.h:20: // A replacement for std::remove_reference until C++11
library support is
On 2015/11/30 21:45:35, scottmg wrote:
> These need to be //! \brief, and indent the second line + 4. Same for comment
on
> remove_reference.

Done.

https://codereview.chromium.org/1483073004/diff/60001/util/misc/move.h#newcode27
util/misc/move.h:27: // A replacement for std::move() until C++11 library
support is available.
On 2015/11/30 21:47:51, Mark Mentovai wrote:
> Does it work to define crashpad::move() and these other things in terms of
> std::move() if CXX_LIBRARY_VERSION >= 2011? Get CXX_LIBRARY_VERSION from
> "util/stdlib/cxx.h".

Sure, we could do that. Idk that it's buying us anything other than making this
harder to read, but done.

[danakj, 2015-11-30 22:10:26]

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:25: using remove_reference = std::remove_reference;
Technically I could avoid making this at all if >= 2011. But I didn't like the
idea of sometimes defining a symbol and sometimes not?

[scottmg, 2015-11-30 22:10:50]

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:25: using remove_reference = std::remove_reference;
FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft
Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
@obj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj.rsp /c
..\..\minidump\minidump_thread_writer_test.cc
/Foobj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj
/Fdobj\minidump\crashpad_minidump_test.cc.pdb
d:\src\crashpad\crashpad\util\stdlib\move.h(25) : error C2955:
'std::remove_reference' : use of class template requires template argument list
        c:\program files (x86)\microsoft visual studio
12.0\vc\include\xtr1common(315) : see declaration of 'std::remove_reference'
ninja: build stopped: subcommand failed.

[Mark Mentovai, 2015-11-30 22:11:41]

LGTM.

I may experiment with injecting this into std in compat so that we can actually
write std::move in Crashpad.

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:39: template<typename T>
I think clang-format likes a space between template and <. Here and on line 46
too.

[danakj, 2015-11-30 22:14:45]

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:25: using remove_reference = std::remove_reference;
On 2015/11/30 22:10:50, scottmg wrote:
> FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files (x86)\Microsoft
> Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
> @obj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj.rsp /c
> ..\..\minidump\minidump_thread_writer_test.cc
> /Foobj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj
> /Fdobj\minidump\crashpad_minidump_test.cc.pdb
> d:\src\crashpad\crashpad\util\stdlib\move.h(25) : error C2955:
> 'std::remove_reference' : use of class template requires template argument
list
>         c:\program files (x86)\microsoft visual studio
> 12.0\vc\include\xtr1common(315) : see declaration of 'std::remove_reference'
> ninja: build stopped: subcommand failed.

mmm.. oh, you can template a typedef as of c++11 apparently. how's this? ^_^'

[danakj, 2015-11-30 22:14:59]

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:39: template<typename T>
On 2015/11/30 22:11:41, Mark Mentovai wrote:
> I think clang-format likes a space between template and <. Here and on line 46
> too.

Done.

[scottmg, 2015-11-30 22:19:55]

lgtm

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
File util/stdlib/move.h (right):

https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
util/stdlib/move.h:25: using remove_reference = std::remove_reference;
On 2015/11/30 22:14:45, danakj (behind on reviews) wrote:
> On 2015/11/30 22:10:50, scottmg wrote:
> > FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files
(x86)\Microsoft
> > Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
> > @obj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj.rsp /c
> > ..\..\minidump\minidump_thread_writer_test.cc
> > /Foobj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj
> > /Fdobj\minidump\crashpad_minidump_test.cc.pdb
> > d:\src\crashpad\crashpad\util\stdlib\move.h(25) : error C2955:
> > 'std::remove_reference' : use of class template requires template argument
> list
> >         c:\program files (x86)\microsoft visual studio
> > 12.0\vc\include\xtr1common(315) : see declaration of 'std::remove_reference'
> > ninja: build stopped: subcommand failed.
> 
> mmm.. oh, you can template a typedef as of c++11 apparently. how's this? ^_^'

We win! (Or, it compiles anyway.)

[danakj, 2015-11-30 22:20:26]

On 2015/11/30 22:19:55, scottmg wrote:
> lgtm
> 
> https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h
> File util/stdlib/move.h (right):
> 
>
https://codereview.chromium.org/1483073004/diff/100001/util/stdlib/move.h#new...
> util/stdlib/move.h:25: using remove_reference = std::remove_reference;
> On 2015/11/30 22:14:45, danakj (behind on reviews) wrote:
> > On 2015/11/30 22:10:50, scottmg wrote:
> > > FAILED: ninja -t msvc -e environment.x86 -- "C:\Program Files
> (x86)\Microsoft
> > > Visual Studio 12.0\VC\bin\amd64_x86\cl.exe" /nologo /showIncludes /FC
> > > @obj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj.rsp
/c
> > > ..\..\minidump\minidump_thread_writer_test.cc
> > > /Foobj\minidump\crashpad_minidump_test.minidump_thread_writer_test.obj
> > > /Fdobj\minidump\crashpad_minidump_test.cc.pdb
> > > d:\src\crashpad\crashpad\util\stdlib\move.h(25) : error C2955:
> > > 'std::remove_reference' : use of class template requires template argument
> > list
> > >         c:\program files (x86)\microsoft visual studio
> > > 12.0\vc\include\xtr1common(315) : see declaration of
'std::remove_reference'
> > > ninja: build stopped: subcommand failed.
> > 
> > mmm.. oh, you can template a typedef as of c++11 apparently. how's this?
^_^'
> 
> We win! (Or, it compiles anyway.)

Thank youuuuuuuuuuu

[danakj, 2015-11-30 22:20:59]

Description was changed from

==========
Replace use of .Pass() with crashpad::move().

Since C++11 library support isn't available everywhere crashpad is
compiled, add our own move() method in the crashpad namespace to replace
std::move() for now. Replace uses of .Pass() with this method.

R=mark@chromium.org, scottmg@chromium.org
BUG=chromium:557422
==========

to

==========
Replace use of .Pass() with crashpad::move().

Since C++11 library support isn't available everywhere crashpad is
compiled, add our own move() method in the crashpad namespace to replace
std::move() for now. Replace uses of .Pass() with this method.

R=mark@chromium.org, scottmg@chromium.org
BUG=chromium:557422

Committed:
https://chromium.googlesource.com/crashpad/crashpad/+/6bebb10829332dee5c7315a...
==========

[danakj, 2015-11-30 22:21:02]

Committed patchset #9 (id:160001) manually as
6bebb10829332dee5c7315abafb0a8bf32840c15 (presubmit successful).

[Mark Mentovai, 2015-11-30 22:49:36]

Thanks, Dana.