Allow explicit conversion operators and implement WTF::OwnPtr::operator bool.

Allow explicit conversion operators and implement WTF::OwnPtr::operator bool.

For consistency, PassOwnPtr::operator bool is also implemented.

This is the simpler, and as of C++11, more idiomatic, way of only allowing
conversion to bool. A minor downside is that the conversion doesn't
automatically take place on return. There are many workarounds, but the !! idiom
seems most common, so that is what has been used here.

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

[jbroman, 2016-04-12 23:19:41]

jbroman@chromium.org changed reviewers:
+ danakj@chromium.org, thakis@chromium.org

[jbroman, 2016-04-12 23:19:42]

Pending discussion on this thread:
https://groups.google.com/a/chromium.org/d/msg/cxx/slLNb2u7Ei4/DLWiG_2wLAAJ

[danakj, 2016-04-12 23:23:24]

LGTM % thakis

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/OwnPtr.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/OwnPtr.h:73: explicit operator bool() const {
return m_ptr; }
do you mean return !!m_ptr?

not sure if this leaks the pointer and people could cast it from bool to pointer
again or weird things?

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/PassOwnPtr.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/PassOwnPtr.h:67: explicit operator bool() const {
return m_ptr; }
ditto?

[jbroman, 2016-04-12 23:26:38]

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/OwnPtr.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/OwnPtr.h:73: explicit operator bool() const {
return m_ptr; }
On 2016/04/12 at 23:23:24, danakj wrote:
> do you mean return !!m_ptr?
> 
> not sure if this leaks the pointer and people could cast it from bool to
pointer again or weird things?

No, conversion to bool will result in either |true| or |false|.
http://en.cppreference.com/w/cpp/language/implicit_conversion#Boolean_convers...

[danakj, 2016-04-12 23:30:12]

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/wtf/OwnPtr.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/wtf/OwnPtr.h:73: explicit operator bool() const {
return m_ptr; }
On 2016/04/12 23:26:38, jbroman wrote:
> On 2016/04/12 at 23:23:24, danakj wrote:
> > do you mean return !!m_ptr?
> > 
> > not sure if this leaks the pointer and people could cast it from bool to
> pointer again or weird things?
> 
> No, conversion to bool will result in either |true| or |false|.
>
http://en.cppreference.com/w/cpp/language/implicit_conversion#Boolean_convers...

Yay thanks

[Yuta Kitamura, 2016-04-13 00:10:24]

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

[Yuta Kitamura, 2016-04-13 00:10:25]

LGTM, this is better :)

(You still need to wait for Nico for wtf approval.)

[Nico, 2016-04-13 01:19:01]

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/InterpolationValue.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/InterpolationValue.h:37: operator
bool() const { return !!interpolableValue; }
bleh, i hate that !! pattern. can we static_cast<bool> instead?

[Nico, 2016-04-13 01:19:59]

(but +1 generally)

[alancutter (OOO until 2018), 2016-04-13 02:26:31]

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

[alancutter (OOO until 2018), 2016-04-13 02:26:32]

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/InterpolationValue.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/InterpolationValue.h:37: operator
bool() const { return !!interpolableValue; }
On 2016/04/13 at 01:19:01, Nico wrote:
> bleh, i hate that !! pattern. can we static_cast<bool> instead?

I dislike !! as well but static_cast<bool> is also undesirable.
I think the practice should be to use .get() instead.

[Nico, 2016-04-13 02:29:35]

If that works that's cool too.
On Apr 12, 2016 10:26 PM, <alancutter@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/animation/InterpolationValue.h
> (right):
>
>
>
https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/animation/InterpolationValue.h:37:
> operator bool() const { return !!interpolableValue; }
> On 2016/04/13 at 01:19:01, Nico wrote:
> > bleh, i hate that !! pattern. can we static_cast<bool> instead?
>
> I dislike !! as well but static_cast<bool> is also undesirable.
> I think the practice should be to use .get() instead.
>
> https://codereview.chromium.org/1878253003/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2016-04-13 02:29:35]

If that works that's cool too.
On Apr 12, 2016 10:26 PM, <alancutter@chromium.org> wrote:

>
>
>
https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/animation/InterpolationValue.h
> (right):
>
>
>
https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/animation/InterpolationValue.h:37:
> operator bool() const { return !!interpolableValue; }
> On 2016/04/13 at 01:19:01, Nico wrote:
> > bleh, i hate that !! pattern. can we static_cast<bool> instead?
>
> I dislike !! as well but static_cast<bool> is also undesirable.
> I think the practice should be to use .get() instead.
>
> https://codereview.chromium.org/1878253003/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[jbroman, 2016-04-13 13:38:09]

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/animation/InterpolationValue.h (right):

https://codereview.chromium.org/1878253003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/animation/InterpolationValue.h:37: operator
bool() const { return !!interpolableValue; }
On 2016/04/13 at 02:26:32, alancutter wrote:
> On 2016/04/13 at 01:19:01, Nico wrote:
> > bleh, i hate that !! pattern. can we static_cast<bool> instead?
> 
> I dislike !! as well but static_cast<bool> is also undesirable.
> I think the practice should be to use .get() instead.

I actually don't mind !!, but static_cast<bool>(pointer) is super verbose. I'm
okay with pointer.get() and relying on the implicit bool conversion there. I
also prefer "pointer != nullptr" (which, amusingly, check-webkit-style actually
rejects) and "bool{pointer}" to static_cast<bool>.

Since .get() seems to be okay with everyone who's weighed in so far, I've
changed to that.

[Nico, 2016-04-13 13:40:38]

lgtm

[jbroman, 2016-04-13 17:42:44]

The CQ bit was checked by jbroman@chromium.org

[jbroman, 2016-04-13 17:42:44]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
yutak@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1878253003/#ps40001
(title: "fix")

[commit-bot: I haz the power, 2016-04-13 17:43:00]

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

[commit-bot: I haz the power, 2016-04-13 18:06:58]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-13 18:07:54]

Description was changed from

==========
Allow explicit conversion operators and implement WTF::OwnPtr::operator bool.

For consistency, PassOwnPtr::operator bool is also implemented.

This is the simpler, and as of C++11, more idiomatic, way of only allowing
conversion to bool. A minor downside is that the conversion doesn't
automatically take place on return. There are many workarounds, but the !! idiom
seems most common, so that is what has been used here.
==========

to

==========
Allow explicit conversion operators and implement WTF::OwnPtr::operator bool.

For consistency, PassOwnPtr::operator bool is also implemented.

This is the simpler, and as of C++11, more idiomatic, way of only allowing
conversion to bool. A minor downside is that the conversion doesn't
automatically take place on return. There are many workarounds, but the !! idiom
seems most common, so that is what has been used here.

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

[commit-bot: I haz the power, 2016-04-13 18:07:56]

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