Remove legacy scoped_ptr implementation.

Remove legacy scoped_ptr implementation.

scoped_ptr is just a type alias now for std::unique_ptr on all platforms.

BUG=554298
Committed: https://crrev.com/ba7fcd6928dbcb974e28fff928e34818ad40bed6
Cr-Commit-Position: refs/heads/master@{#381132}

[dcheng, 2016-03-14 18:31:59]

dcheng@chromium.org changed reviewers:
+ danakj@chromium.org

[dcheng, 2016-03-14 18:31:59]

Whee. I'll clean up the headers slowly after this.

[danakj, 2016-03-14 18:34:09]

Amaaaazing. LGTM.

Please PSA once this lands, people should start writing unique_ptr.

[dcheng, 2016-03-14 19:40:31]

On 2016/03/14 at 18:34:09, danakj wrote:
> Amaaaazing. LGTM.
> 
> Please PSA once this lands, people should start writing unique_ptr.

Hmm, do we want to encourage that? Are we concerned about the missing overloads
from libstdc++ for comparing nullptr_t and std::unique_ptr? I was originally
thinking that we might want to wait until we have libstdc++4.8 or so.

If they are important, then we should tell people to keep using scoped_ptr,
because these operators will randomly break depending on if people include
<memory> or <scoped_ptr.h>.

If they aren't important, then maybe we should just delete them: they're not
called except by the unit test, std::map won't use it anyway (since nullptr_t
should convert to a std::unique_ptr before comparisons), and everyone that uses
std::unique_ptr can just consistently include the same thing.

Also, WDYT of just deleting the unit tests for scoped_ptr as well?

[Nico, 2016-03-14 19:43:04]

thakis@chromium.org changed reviewers:
+ thakis@chromium.org

[Nico, 2016-03-14 19:43:04]

maybe we can put the overloads in a separate file (base/unique_ptr_comparison.h
or so), include that from this header, and tell people to include that if they
need the comparison.

I'm assuming we'll also s/scoped_ptr/std::unique_ptr/g at some point after this
is in, right?

Thanks much for helping with this, exciting to see the progress :-)

[dcheng, 2016-03-14 19:48:53]

On 2016/03/14 at 19:43:04, thakis wrote:
> maybe we can put the overloads in a separate file
(base/unique_ptr_comparison.h or so), include that from this header, and tell
people to include that if they need the comparison.
> 
> I'm assuming we'll also s/scoped_ptr/std::unique_ptr/g at some point after
this is in, right?
> 
> Thanks much for helping with this, exciting to see the progress :-)

We could, but I'm not sure if there's much value in doing so.

- std::set/std::map will never use comparison operators for nullptr_t and
std::unique_ptr.
- you could write if (nullptr < my_unique_ptr), but you may as well just write
if (my_unique_ptr)
- Dana points out that templated code might but there are zero instances of that
actually happening in Chrome, as far as I can tell.

[dcheng, 2016-03-14 19:50:57]

On 2016/03/14 at 19:43:04, thakis wrote:
> maybe we can put the overloads in a separate file
(base/unique_ptr_comparison.h or so), include that from this header, and tell
people to include that if they need the comparison.
> 
> I'm assuming we'll also s/scoped_ptr/std::unique_ptr/g at some point after
this is in, right?
> 
> Thanks much for helping with this, exciting to see the progress :-)

And yeah, search and replace should be pretty easy after this. Likely it'll be
something like:
1. Pare down list of #includes in scoped_ptr.h to just <memory>.
2. Search and replace and commit. I'm not sure if it's worth breaking this up
into multiple patches: I'm thinking one giant patch might be nicer in terms of
consistency, and it should be fairly landable over a weekend.
3. ???
4. Profit!

[dcheng, 2016-03-14 20:00:35]

Also, an argument against adding unique_ptr_comparisons.h: at the very least,
it's going to have to include cstddef and <memory>. It's unlikely this header
will ever be needed/used, but if it does get #included in some fairly public
location (say, some content/public interface), then it could become a pain to
remove the unique_ptr_comparisons header down the road due to IWYU violations.

In short: if someone ever does need these comparison operators, it's not hard to
add it then. Until that need is demonstrated, we save ourselves from having to
fix (more) IWYU problems down the road.

[Nico, 2016-03-14 21:41:32]

If it's not needed anywhere, then I guess let's remove the operators too in a
follow-up. (I guess removing this file in multiple changes is a good idea
anyhow, so I'm not 100% sure how silly I should feel for asking to keep them
around a while ago :-P)

[dcheng, 2016-03-14 22:30:21]

On 2016/03/14 at 21:41:32, thakis wrote:
> If it's not needed anywhere, then I guess let's remove the operators too in a
follow-up. (I guess removing this file in multiple changes is a good idea
anyhow, so I'm not 100% sure how silly I should feel for asking to keep them
around a while ago :-P)

OK, updated this CL to restore the operators for now. PTAL.

[Nico, 2016-03-14 22:31:32]

lgtm+++++++++++++++

[dcheng, 2016-03-14 22:34:59]

The CQ bit was checked by dcheng@chromium.org

[dcheng, 2016-03-14 22:35:00]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/1801823003/#ps60001
(title: "Restore operators")

[commit-bot: I haz the power, 2016-03-14 22:35:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1801823003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1801823003/60001

[commit-bot: I haz the power, 2016-03-15 00:29:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-15 00:30:28]

Description was changed from

==========
Remove legacy scoped_ptr implementation.

scoped_ptr is just a type alias now for std::unique_ptr on all platforms.

BUG=554298
==========

to

==========
Remove legacy scoped_ptr implementation.

scoped_ptr is just a type alias now for std::unique_ptr on all platforms.

BUG=554298

Committed: https://crrev.com/ba7fcd6928dbcb974e28fff928e34818ad40bed6
Cr-Commit-Position: refs/heads/master@{#381132}
==========

[commit-bot: I haz the power, 2016-03-15 00:30:29]

Patchset 4 (id:??) landed as
https://crrev.com/ba7fcd6928dbcb974e28fff928e34818ad40bed6
Cr-Commit-Position: refs/heads/master@{#381132}