629289 - forcing value to bool (performance warning C4800)

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 1

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 507365 [details] [diff] [review]
patch

I'm not sure this patch actually fixes perf right? There is still a !! conversion. Does just adding !! fix the warning?

Comment 2

[alexander :surkov (:asurkov)]

(In reply to comment #1)
> Comment on attachment 507365 [details] [diff] [review]
> patch
> 
> I'm not sure this patch actually fixes perf right?

there is I think, implicit conversion costs something.

> There is still a !!
> conversion.

for the second case

> Does just adding !! fix the warning?

yes, perhaps static_cast<bool> is better here, dunno

Comment 3

[alexander :surkov (:asurkov)]

Neil, what's better:

bool IsBoundToParent() const { return !!mParent; }

or

bool IsBoundToParent() const { return static_cast<bool>(mParent); }

Comment 4

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #3)
> bool IsBoundToParent() const { return static_cast<bool>(mParent); }

Neil tells me this won't remove the warning.

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 507365 [details] [diff] [review]
patch

r+a=me, we probably don't want to take two many low priority bugs like this until after 4 ships. Can you ride this patch along one of your pushes to try to make sure we don't have an freaky failures?

Comment 6

[alexander :surkov (:asurkov)]

landed on 2.0 - http://hg.mozilla.org/mozilla-central/rev/a2d89cf5cd18