cc: Remove use of PassAs() and constructor-casting with scoped_ptr.

cc: Remove use of PassAs() and constructor-casting with scoped_ptr.

Say you have class A and subclass B.

Previously it was required to PassAs() a scoped_ptr<B> into a
scoped_ptr<A>. This is no longer needed, so just use Pass(). For newly
created scoped_ptrs, you can just use make_scoped_ptr always now.

And when you want to return or assign an empty scoped_ptr(), you can
now use nullptr directly.

Also adds PRESUBMIT checks for:
- return scoped<T>(foo). This should be return make_scoped_ptr(foo).
- bar = scoped<T>(foo). This should be return bar = make_scoped_ptr(foo).
- return scoped<T>(). This should be return nullptr.
- bar = scoped<T>(). This should be return bar = nullptr.

This also replaces p.reset() with p = nullptr; But it does not add a
PRESUBMIT check for that because there are things other than scoped_ptr
with a reset() function.

R=enne@chromium.org

Committed: https://crrev.com/7bb3dbede19d87f0338797756ffd738adc6bca08
Cr-Commit-Position: refs/heads/master@{#297096}

Committed: https://crrev.com/f446a070a0aa29a153b0cf78b33ef22da84cb023
Cr-Commit-Position: refs/heads/master@{#297121}

[danakj, 2014-09-26 04:08:49]

Patchset #1 (id:1) has been deleted

[danakj, 2014-09-26 04:17:08]

This includes the base changes so we can see what the bots make of it, those
will rebase away.

[danakj, 2014-09-26 04:18:08]

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

[danakj, 2014-09-26 04:18:09]

or +vmpstr if he's bored :3

[jamesr, 2014-09-26 04:35:51]

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

[jamesr, 2014-09-26 04:35:52]

Can you upload a patch that doesn't include the base/ bits?

[danakj, 2014-09-26 13:21:28]

On 2014/09/26 04:35:52, jamesr wrote:
> Can you upload a patch that doesn't include the base/ bits?

Ya, I just want to see green trybots on all platforms first to make sure this
works.

[danakj, 2014-09-26 15:19:51]

On 2014/09/26 13:21:28, danakj wrote:
> On 2014/09/26 04:35:52, jamesr wrote:
> > Can you upload a patch that doesn't include the base/ bits?
> 
> Ya, I just want to see green trybots on all platforms first to make sure this
> works.

Patchset 3 has green bots with the base change. Patchset 4 is without the base
change (won't compile on TOT).

[vmpstr, 2014-09-26 16:44:23]

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py
File cc/PRESUBMIT.py (right):

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py#newcode171
cc/PRESUBMIT.py:171: if re.search(r'(=|\breturn)\s*scoped_ptr<[^>+]>\(\)',
line):
... [^>]+ ...

?

https://codereview.chromium.org/609663003/diff/80001/cc/animation/keyframed_a...
File cc/animation/keyframed_animation_curve.cc (left):

https://codereview.chromium.org/609663003/diff/80001/cc/animation/keyframed_a...
cc/animation/keyframed_animation_curve.cc:272:
scoped_ptr<KeyframedTransformAnimationCurve> to_return(
Are we not allowing this type of constructor either?

[danakj, 2014-09-26 16:56:44]

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py
File cc/PRESUBMIT.py (right):

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py#newcode171
cc/PRESUBMIT.py:171: if re.search(r'(=|\breturn)\s*scoped_ptr<[^>+]>\(\)',
line):
On 2014/09/26 16:44:23, vmpstr wrote:
> ... [^>]+ ...
> 
> ?

That matches the T in scoped_ptr<T>

It doesn't handle scoped_ptr<T<U>> though.. if we care.

https://codereview.chromium.org/609663003/diff/80001/cc/animation/keyframed_a...
File cc/animation/keyframed_animation_curve.cc (left):

https://codereview.chromium.org/609663003/diff/80001/cc/animation/keyframed_a...
cc/animation/keyframed_animation_curve.cc:272:
scoped_ptr<KeyframedTransformAnimationCurve> to_return(
On 2014/09/26 16:44:23, vmpstr wrote:
> Are we not allowing this type of constructor either?

No, it's fine, I just think following matching () across lines is a bit more
annoying than following a = across a line break. So I instinctively changed it,
heh.

[enne (OOO), 2014-09-26 16:58:22]

lgtm

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py
File cc/PRESUBMIT.py (right):

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py#newcode164
cc/PRESUBMIT.py:164: if
re.search(r'(=|\breturn)\s*scoped_ptr<[^>]*[^>\]]>\([^)]+\)', line):
regex nits:

matching non-greedily, [^>]*> is clearer as .*?>
don't end with a character, [^.\]] could just be (?<!])

https://codereview.chromium.org/609663003/diff/80001/cc/PRESUBMIT.py#newcode171
cc/PRESUBMIT.py:171: if re.search(r'(=|\breturn)\s*scoped_ptr<[^>+]>\(\)',
line):
I think this is wrong; did you intend for the + to be outside of the [] here?
Also, this could probably just be scoped_ptr<.*?> as well.

[danakj, 2014-09-26 17:06:28]

OOh I see. Ok fixed up regexes! Thanks!

[danakj, 2014-09-26 23:47:25]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-26 23:47:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/609663003/100001

[danakj, 2014-09-27 00:39:01]

Added a PRESUBMIT warning for PassAs() as well!

[danakj, 2014-09-27 00:39:16]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-27 00:40:12]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/609663003/120001

[enne (OOO), 2014-09-27 00:45:14]

lgtm, thanks for adding that.  :) :)

[dcheng, 2014-09-27 00:56:59]

On 2014/09/27 at 00:45:14, enne wrote:
> lgtm, thanks for adding that.  :) :)

Just a random question--is there any reason to prefer x = nullptr; to x.reset()?

[Peter Kasting, 2014-09-27 00:59:45]

On 2014/09/27 00:56:59, dcheng wrote:
> On 2014/09/27 at 00:45:14, enne wrote:
> > lgtm, thanks for adding that.  :) :)
> 
> Just a random question--is there any reason to prefer x = nullptr; to
x.reset()?

That was my question as well -- it's the only one of these changes that seems
potentially iffy.

One might argue that "x.reset()" is more obvious that x is not a raw pointer, if
the reader happens to not have the context of precisely what ype |x| is paged
into their head.

[jamesr, 2014-09-27 01:02:09]

I'd argue that when doing "x = nullptr;" the reader shouldn't care what the type
of x is, since it always means the same thing.

[Peter Kasting, 2014-09-27 01:04:08]

On 2014/09/27 01:02:09, jamesr wrote:
> I'd argue that when doing "x = nullptr;" the reader shouldn't care what the
type
> of x is, since it always means the same thing.

Well, one means "delete the old value of x", and the other doesn't.

[danakj, 2014-09-27 01:06:58]

On Fri, Sep 26, 2014 at 9:04 PM, <pkasting@chromium.org> wrote:

> On 2014/09/27 01:02:09, jamesr wrote:
>
>> I'd argue that when doing "x = nullptr;" the reader shouldn't care what
>> the
>>
> type
>
>> of x is, since it always means the same thing.
>>
>
> Well, one means "delete the old value of x", and the other doesn't.
>

I learnt afterward that unique_ptr() also has a reset() that defaults to
nullptr. I think they're about equivalent, but one looks like changing a
pointer to null, and one looks like calling a method on some object,
nullptr maybe slightly more obvious what's going on. Maybe.

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-09-27 02:39:44]

Committed patchset #6 (id:120001) as 698209acda4f8da4d272e802643ef84259cd02c9

[commit-bot: I haz the power, 2014-09-27 02:40:16]

Patchset 6 (id:??) landed as
https://crrev.com/7bb3dbede19d87f0338797756ffd738adc6bca08
Cr-Commit-Position: refs/heads/master@{#297096}

[dcheng, 2014-09-27 07:31:18]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/608503005/ by dcheng@chromium.org.

The reason for reverting is: scoped_ptr nullptr support needs to be reverted.

[danakj, 2014-09-27 21:53:42]

The CQ bit was checked by danakj@chromium.org

[commit-bot: I haz the power, 2014-09-27 21:54:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/609663003/120001

[commit-bot: I haz the power, 2014-09-27 21:56:03]

Committed patchset #6 (id:120001) as f12498e35111694644bec64a5ff0521398147335

[commit-bot: I haz the power, 2014-09-27 21:56:40]

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