Add DoubleEqualsInt32 which accepts negative zero

RESOLVED FIXED in mozilla28

Status

()

enhancement
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: sunfish, Assigned: sunfish)

Tracking

unspecified
mozilla28
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

Assignee

Description

6 years ago
The attached patch adds a DoubleEqualsInt32 utility, which is similar to DoubleIsInt32 except that it considers negative zero to be equal to zero. This has a variety of uses, include range analysis' beta node insertion, and several places which were doing a manual conversion with an equality check.

DoubleEqualsInt32 is better than manual code because converting a double to int32 can technically invoke undefined behavior --- see the existing XXX comment in DoubleIsInt32. With this patch, this is done in one place, with said comment, rather than in several places, which is an improvement.
Attachment #821919 - Flags: review?(nicolas.b.pierron)
Comment on attachment 821919 [details] [diff] [review]
range-double-equals-int32.patch

Review of attachment 821919 [details] [diff] [review]:
-----------------------------------------------------------------

Sounds good to me.
I put Waldo in the feedback loop.
Attachment #821919 - Flags: review?(nicolas.b.pierron)
Attachment #821919 - Flags: review+
Attachment #821919 - Flags: feedback?(jwalden+bmo)
Comment on attachment 821919 [details] [diff] [review]
range-double-equals-int32.patch

Review of attachment 821919 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsmath.cpp
@@ +40,5 @@
>  using namespace js;
>  
>  using mozilla::Abs;
>  using mozilla::DoubleIsInt32;
> +using mozilla::DoubleEqualsInt32;

E before I, so one line earlier.
Attachment #821919 - Flags: feedback?(jwalden+bmo) → feedback+
Assignee

Comment 3

6 years ago
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #2)
> Comment on attachment 821919 [details] [diff] [review]
> range-double-equals-int32.patch
> 
> Review of attachment 821919 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsmath.cpp
> @@ +40,5 @@
> >  using namespace js;
> >  
> >  using mozilla::Abs;
> >  using mozilla::DoubleIsInt32;
> > +using mozilla::DoubleEqualsInt32;
> 
> E before I, so one line earlier.

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/592b05772531
https://hg.mozilla.org/mozilla-central/rev/592b05772531
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28

Updated

5 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.