Bug 1459611 - Use NumberEqualsInt32 when negative and positive zero are treated the same. r=jandem
authorAndré Bargull <andre.bargull@gmail.com>
Mon, 07 May 2018 09:28:12 -0700
changeset 471681 ce4fdee191230038af2188d9c9ed1ee126fa551f
parent 471680 5f691b58a680400323d78bebeb22dff9b0f954b2
child 471682 06bee42b4d4955429061f706bd59b8275d8e77f4
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1459611
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1459611 - Use NumberEqualsInt32 when negative and positive zero are treated the same. r=jandem
js/src/builtin/JSON.cpp
js/src/frontend/BytecodeEmitter.cpp
js/src/jsmath.cpp
js/src/jsnum.cpp
js/src/jsnum.h
js/src/vm/JSAtom-inl.h
--- a/js/src/builtin/JSON.cpp
+++ b/js/src/builtin/JSON.cpp
@@ -656,47 +656,29 @@ js::Stringify(JSContext* cx, MutableHand
             for (; k < len; k++) {
                 if (!CheckForInterrupt(cx))
                     return false;
 
                 /* Step 4b(iii)(5)(a-b). */
                 if (!GetElement(cx, replacer, k, &item))
                     return false;
 
-                RootedId id(cx);
+                /* Step 4b(iii)(5)(c-g). */
+                if (!item.isNumber() && !item.isString()) {
+                    ESClass cls;
+                    if (!GetClassOfValue(cx, item, &cls))
+                        return false;
 
-                /* Step 4b(iii)(5)(c-f). */
-                if (item.isNumber()) {
-                    /* Step 4b(iii)(5)(e). */
-                    int32_t n;
-                    if (ValueFitsInInt32(item, &n) && INT_FITS_IN_JSID(n)) {
-                        id = INT_TO_JSID(n);
-                    } else {
-                        if (!ValueToId<CanGC>(cx, item, &id))
-                            return false;
-                    }
-                } else {
-                    bool shouldAdd = item.isString();
-                    if (!shouldAdd) {
-                        ESClass cls;
-                        if (!GetClassOfValue(cx, item, &cls))
-                            return false;
+                    if (cls != ESClass::String && cls != ESClass::Number)
+                        continue;
+                }
 
-                        shouldAdd = cls == ESClass::String || cls == ESClass::Number;
-                    }
-
-                    if (shouldAdd) {
-                        /* Step 4b(iii)(5)(f). */
-                        if (!ValueToId<CanGC>(cx, item, &id))
-                            return false;
-                    } else {
-                        /* Step 4b(iii)(5)(g). */
-                        continue;
-                    }
-                }
+                RootedId id(cx);
+                if (!ValueToId<CanGC>(cx, item, &id))
+                    return false;
 
                 /* Step 4b(iii)(5)(g). */
                 auto p = idSet.lookupForAdd(id);
                 if (!p) {
                     /* Step 4b(iii)(5)(g)(i). */
                     if (!idSet.add(p, id) || !propertyList.append(id))
                         return false;
                 }
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -44,16 +44,17 @@
 using namespace js;
 using namespace js::gc;
 using namespace js::frontend;
 
 using mozilla::AssertedCast;
 using mozilla::DebugOnly;
 using mozilla::Maybe;
 using mozilla::Nothing;
+using mozilla::NumberEqualsInt32;
 using mozilla::NumberIsInt32;
 using mozilla::PodCopy;
 using mozilla::Some;
 using mozilla::Unused;
 
 class BreakableControl;
 class LabelControl;
 class LoopControl;
@@ -4664,17 +4665,17 @@ BytecodeEmitter::emitSwitch(ParseNode* p
             ParseNode* caseValue = caseNode->caseExpression();
 
             if (caseValue->getKind() != ParseNodeKind::Number) {
                 switchOp = JSOP_CONDSWITCH;
                 continue;
             }
 
             int32_t i;
-            if (!NumberIsInt32(caseValue->pn_dval, &i)) {
+            if (!NumberEqualsInt32(caseValue->pn_dval, &i)) {
                 switchOp = JSOP_CONDSWITCH;
                 continue;
             }
 
             if (unsigned(i + int(JS_BIT(15))) >= unsigned(JS_BIT(16))) {
                 switchOp = JSOP_CONDSWITCH;
                 continue;
             }
--- a/js/src/jsmath.cpp
+++ b/js/src/jsmath.cpp
@@ -840,34 +840,34 @@ template double js::GetBiggestNumberLess
 template float js::GetBiggestNumberLessThan<>(float x);
 
 double
 js::math_round_impl(double x)
 {
     AutoUnsafeCallWithABI unsafe;
 
     int32_t ignored;
-    if (NumberIsInt32(x, &ignored))
+    if (NumberEqualsInt32(x, &ignored))
         return x;
 
     /* Some numbers are so big that adding 0.5 would give the wrong number. */
     if (ExponentComponent(x) >= int_fast16_t(FloatingPoint<double>::kExponentShift))
         return x;
 
     double add = (x >= 0) ? GetBiggestNumberLessThan(0.5) : 0.5;
     return std::copysign(fdlibm::floor(x + add), x);
 }
 
 float
 js::math_roundf_impl(float x)
 {
     AutoUnsafeCallWithABI unsafe;
 
     int32_t ignored;
-    if (NumberIsInt32(x, &ignored))
+    if (NumberEqualsInt32(x, &ignored))
         return x;
 
     /* Some numbers are so big that adding 0.5 would give the wrong number. */
     if (ExponentComponent(x) >= int_fast16_t(FloatingPoint<float>::kExponentShift))
         return x;
 
     float add = (x >= 0) ? GetBiggestNumberLessThan(0.5f) : 0.5f;
     return std::copysign(fdlibm::floorf(x + add), x);
--- a/js/src/jsnum.cpp
+++ b/js/src/jsnum.cpp
@@ -1277,17 +1277,17 @@ js::InitNumberClass(JSContext* cx, Handl
 }
 
 static char*
 FracNumberToCString(JSContext* cx, ToCStringBuf* cbuf, double d, int base = 10)
 {
 #ifdef DEBUG
     {
         int32_t _;
-        MOZ_ASSERT(!mozilla::NumberIsInt32(d, &_));
+        MOZ_ASSERT(!mozilla::NumberEqualsInt32(d, &_));
     }
 #endif
 
     char* numStr;
     if (base == 10) {
         /*
          * This is V8's implementation of the algorithm described in the
          * following paper:
@@ -1308,41 +1308,35 @@ FracNumberToCString(JSContext* cx, ToCSt
     return numStr;
 }
 
 char*
 js::NumberToCString(JSContext* cx, ToCStringBuf* cbuf, double d, int base/* = 10*/)
 {
     int32_t i;
     size_t len;
-    return mozilla::NumberIsInt32(d, &i)
+    return mozilla::NumberEqualsInt32(d, &i)
            ? Int32ToCString(cbuf, i, &len, base)
            : FracNumberToCString(cx, cbuf, d, base);
 }
 
 template <AllowGC allowGC>
 static JSString*
 NumberToStringWithBase(JSContext* cx, double d, int base)
 {
+    MOZ_ASSERT(2 <= base && base <= 36);
+
     ToCStringBuf cbuf;
     char* numStr;
 
-    /*
-     * Caller is responsible for error reporting. When called from trace,
-     * returning nullptr here will cause us to fall of trace and then retry
-     * from the interpreter (which will report the error).
-     */
-    if (base < 2 || base > 36)
-        return nullptr;
-
     JSCompartment* comp = cx->compartment();
 
     int32_t i;
     bool isBase10Int = false;
-    if (mozilla::NumberIsInt32(d, &i)) {
+    if (mozilla::NumberEqualsInt32(d, &i)) {
         isBase10Int = (base == 10);
         if (isBase10Int && StaticStrings::hasInt(i))
             return cx->staticStrings().getInt(i);
         if (unsigned(i) < unsigned(base)) {
             if (i < 10)
                 return cx->staticStrings().getInt(i);
             char16_t c = 'a' + i - 10;
             MOZ_ASSERT(StaticStrings::hasUnit(c));
@@ -1393,17 +1387,17 @@ js::NumberToString<CanGC>(JSContext* cx,
 
 template JSString*
 js::NumberToString<NoGC>(JSContext* cx, double d);
 
 JSAtom*
 js::NumberToAtom(JSContext* cx, double d)
 {
     int32_t si;
-    if (mozilla::NumberIsInt32(d, &si))
+    if (mozilla::NumberEqualsInt32(d, &si))
         return Int32ToAtom(cx, si);
 
     if (JSFlatString* str = LookupDtoaCache(cx, d))
         return AtomizeString(cx, str);
 
     ToCStringBuf cbuf;
     char* numStr = FracNumberToCString(cx, &cbuf, d);
     if (!numStr) {
--- a/js/src/jsnum.h
+++ b/js/src/jsnum.h
@@ -196,26 +196,16 @@ js_strtod(JSContext* cx, const CharT* be
 namespace js {
 
 extern MOZ_MUST_USE bool
 num_toString(JSContext* cx, unsigned argc, Value* vp);
 
 extern MOZ_MUST_USE bool
 num_valueOf(JSContext* cx, unsigned argc, Value* vp);
 
-static MOZ_ALWAYS_INLINE bool
-ValueFitsInInt32(const Value& v, int32_t* pi)
-{
-    if (v.isInt32()) {
-        *pi = v.toInt32();
-        return true;
-    }
-    return v.isDouble() && mozilla::NumberIsInt32(v.toDouble(), pi);
-}
-
 /*
  * Returns true if the given value is definitely an index: that is, the value
  * is a number that's an unsigned 32-bit integer.
  *
  * This method prioritizes common-case speed over accuracy in every case.  It
  * can produce false negatives (but not false positives): some values which are
  * indexes will be reported not to be indexes by this method.  Users must
  * consider this possibility when using this method.
@@ -224,17 +214,17 @@ static MOZ_ALWAYS_INLINE bool
 IsDefinitelyIndex(const Value& v, uint32_t* indexp)
 {
     if (v.isInt32() && v.toInt32() >= 0) {
         *indexp = v.toInt32();
         return true;
     }
 
     int32_t i;
-    if (v.isDouble() && mozilla::NumberIsInt32(v.toDouble(), &i) && i >= 0) {
+    if (v.isDouble() && mozilla::NumberEqualsInt32(v.toDouble(), &i) && i >= 0) {
         *indexp = uint32_t(i);
         return true;
     }
 
     if (v.isString() && v.toString()->hasIndexValue()) {
         *indexp = v.toString()->getIndexValue();
         return true;
     }
--- a/js/src/vm/JSAtom-inl.h
+++ b/js/src/vm/JSAtom-inl.h
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef vm_JSAtom_inl_h
 #define vm_JSAtom_inl_h
 
 #include "vm/JSAtom.h"
 
+#include "mozilla/FloatingPoint.h"
 #include "mozilla/RangedPtr.h"
 
 #include "jsnum.h"
 
 #include "vm/Runtime.h"
 #include "vm/StringType.h"
 
 namespace js {
@@ -29,32 +30,45 @@ AtomToId(JSAtom* atom)
 
     return JSID_FROM_BITS(size_t(atom));
 }
 
 // Use the NameToId method instead!
 inline jsid
 AtomToId(PropertyName* name) = delete;
 
+MOZ_ALWAYS_INLINE bool
+ValueToIntId(const Value& v, jsid* id)
+{
+    int32_t i;
+    if (v.isInt32())
+        i = v.toInt32();
+    else if (!v.isDouble() || !mozilla::NumberEqualsInt32(v.toDouble(), &i))
+        return false;
+
+    if (!INT_FITS_IN_JSID(i))
+        return false;
+
+    *id = INT_TO_JSID(i);
+    return true;
+}
+
 inline bool
 ValueToIdPure(const Value& v, jsid* id)
 {
     if (v.isString()) {
         if (v.toString()->isAtom()) {
             *id = AtomToId(&v.toString()->asAtom());
             return true;
         }
         return false;
     }
 
-    int32_t i;
-    if (ValueFitsInInt32(v, &i) && INT_FITS_IN_JSID(i)) {
-        *id = INT_TO_JSID(i);
+    if (ValueToIntId(v, id))
         return true;
-    }
 
     if (v.isSymbol()) {
         *id = SYMBOL_TO_JSID(v.toSymbol());
         return true;
     }
 
     return false;
 }
@@ -65,21 +79,18 @@ ValueToId(JSContext* cx, typename MaybeR
           typename MaybeRooted<jsid, allowGC>::MutableHandleType idp)
 {
     if (v.isString()) {
         if (v.toString()->isAtom()) {
             idp.set(AtomToId(&v.toString()->asAtom()));
             return true;
         }
     } else {
-        int32_t i;
-        if (ValueFitsInInt32(v, &i) && INT_FITS_IN_JSID(i)) {
-            idp.set(INT_TO_JSID(i));
+        if (ValueToIntId(v, idp.address()))
             return true;
-        }
 
         if (v.isSymbol()) {
             idp.set(SYMBOL_TO_JSID(v.toSymbol()));
             return true;
         }
     }
 
     JSAtom* atom = ToAtom<allowGC>(cx, v);