Allow scoped_ptr variables to be used in boolean expressions

Allow scoped_ptr variables to be used in boolean expressions

BUG=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=160570

[enne (OOO), 2012-10-04 23:36:25]

I find it tedious to have to .get() any scoped_ptr variable that I want to use
in a conditional statement, e.g.:

    scoped_ptr<foo> x;
    if (x.get()) { /* stuff */ }

With this patch, this is now legal:

    scoped_ptr<foo> x;
    if (x) { /* stuff */ }

Reference: http://www.artima.com/cppsource/safebool.html

[tfarina, 2012-10-04 23:44:01]

This is helpful for converting from OwnPtr to scoped_ptr in cc/, since OwnPtr
seems to allow this test.

[enne (OOO), 2012-10-05 17:39:59]

/me adds a reviewer who is actually an OWNER.

[willchan no longer on Chromium, 2012-10-05 21:14:10]

+jyasskin

Jeffrey, will unique_ptr support this? If not, I'm not inclined to approve
this, since our long-term plan is to switch to std::unique_ptr instead of
scoped_ptr.


On Fri, Oct 5, 2012 at 10:39 AM, <enne@chromium.org> wrote:

> /me adds a reviewer who is actually an OWNER.
>
>
https://codereview.chromium.**org/11028044/<https://codereview.chromium.org/1...
>

[tfarina, 2012-10-05 21:16:32]

On Fri, Oct 5, 2012 at 6:14 PM, William Chan (陈智昌)
<willchan@chromium.org> wrote:
> +jyasskin
>
> Jeffrey, will unique_ptr support this? If not, I'm not inclined to approve
> this, since our long-term plan is to switch to std::unique_ptr instead of
> scoped_ptr.
>
It seems so.

http://en.cppreference.com/w/cpp/memory/unique_ptr

-- 
Thiago

[Jeffrey Yasskin, 2012-10-05 21:26:06]

On Fri, Oct 5, 2012 at 2:14 PM, William Chan (陈智昌)
<willchan@chromium.org> wrote:
> +jyasskin
>
> Jeffrey, will unique_ptr support this? If not, I'm not inclined to approve
> this, since our long-term plan is to switch to std::unique_ptr instead of
> scoped_ptr.

Yes, unique_ptr has an explicit operator bool:
[unique.ptr.single.observers]. (And via Thiago's link:
http://en.cppreference.com/w/cpp/memory/unique_ptr/operator_bool)

> On Fri, Oct 5, 2012 at 10:39 AM, <enne@chromium.org> wrote:
>>
>> /me adds a reviewer who is actually an OWNER.
>>
>> https://codereview.chromium.org/11028044/
>
>

[willchan no longer on Chromium, 2012-10-05 21:47:41]

I'm headed into the office and will sanity check the CL then.
On Oct 5, 2012 2:26 PM, "Jeffrey Yasskin" <jyasskin@chromium.org> wrote:

> On Fri, Oct 5, 2012 at 2:14 PM, William Chan (陈智昌)
> <willchan@chromium.org> wrote:
> > +jyasskin
> >
> > Jeffrey, will unique_ptr support this? If not, I'm not inclined to
> approve
> > this, since our long-term plan is to switch to std::unique_ptr instead of
> > scoped_ptr.
>
> Yes, unique_ptr has an explicit operator bool:
> [unique.ptr.single.observers]. (And via Thiago's link:
> http://en.cppreference.com/w/cpp/memory/unique_ptr/operator_bool)
>
> > On Fri, Oct 5, 2012 at 10:39 AM, <enne@chromium.org> wrote:
> >>
> >> /me adds a reviewer who is actually an OWNER.
> >>
> >> https://codereview.chromium.org/11028044/
> >
> >
>

[willchan no longer on Chromium, 2012-10-05 22:19:13]

Just have a few nits.

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h
File base/memory/scoped_ptr.h (right):

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:207: // implicitly convertable to a real bool (which is
dangerous).
s/convertable/convertible/

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:208: typedef C* scoped_ptr::*testable;
Since testable is a type, please name it Testable.

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:209: operator testable() const { return ptr_ ?
&scoped_ptr::ptr_ : 0; }
We use NULL, a la Google C++ Style Guide.

[willchan no longer on Chromium, 2012-10-05 22:21:53]

Jeffrey offers the great suggestion of conditionally compiling with explicit
operator bool() in C++11 mode. You shouldn't do that here, but we should
consider that in a future CL. I need to check on the status of our C++11 build.

After you commit this, please announce your change in chromium-dev@, since
people will probably be glad to avoid the .get() annoyance.

Also, just to clarify my approval, I did it because we aren't losing much by
further separating from backwards compatibility with google3 scoped_ptr, since
they are switching to unique_ptr anyway, which we are moving toward too.

[enne (OOO), 2012-10-06 02:18:41]

Thanks, I'll send an email to chromium-dev once this lands.

If only C++11x constructs worked everywhere...</wistful>

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h
File base/memory/scoped_ptr.h (right):

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:207: // implicitly convertable to a real bool (which is
dangerous).
On 2012/10/05 22:19:13, willchan wrote:
> s/convertable/convertible/

Done. *blush*

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:208: typedef C* scoped_ptr::*testable;
On 2012/10/05 22:19:13, willchan wrote:
> Since testable is a type, please name it Testable.

Done.

https://codereview.chromium.org/11028044/diff/1/base/memory/scoped_ptr.h#newc...
base/memory/scoped_ptr.h:209: operator testable() const { return ptr_ ?
&scoped_ptr::ptr_ : 0; }
On 2012/10/05 22:19:13, willchan wrote:
> We use NULL, a la Google C++ Style Guide.

Done.

[willchan no longer on Chromium, 2012-10-06 02:20:17]

lgtm

[commit-bot: I haz the power, 2012-10-06 03:22:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/enne@chromium.org/11028044/11001

[commit-bot: I haz the power, 2012-10-06 07:47:13]

Change committed as 160570