Bug 1132045 - Add handles to various equality operations. r=jandem
authorTom Schuster <evilpies@gmail.com>
Tue, 17 Feb 2015 15:03:23 +0100
changeset 229460 e00ed1b014d0047b2a3ac15fa563b8f3019f2b16
parent 229459 70d0d6ba5e9c89e136d587f2bfd5ce56795e112d
child 229461 51c2796056eab4b61a33e719c603e0602451f0a1
push id28287
push userryanvm@gmail.com
push dateTue, 17 Feb 2015 20:08:22 +0000
treeherdermozilla-central@b6c56fab513d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1132045
milestone38.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 1132045 - Add handles to various equality operations. r=jandem
js/src/builtin/MapObject.cpp
js/src/jsapi-tests/testSameValue.cpp
js/src/jsapi-tests/testValueABI.cpp
js/src/jsapi.cpp
js/src/jsapi.h
js/src/jsobj.cpp
js/src/vm/Interpreter.cpp
js/src/vm/Interpreter.h
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -818,17 +818,20 @@ HashableValue::hash() const
 bool
 HashableValue::operator==(const HashableValue &other) const
 {
     // Two HashableValues are equal if they have equal bits.
     bool b = (value.asRawBits() == other.value.asRawBits());
 
 #ifdef DEBUG
     bool same;
-    MOZ_ASSERT(SameValue(nullptr, value, other.value, &same));
+    PerThreadData *data = TlsPerThreadData.get();
+    RootedValue valueRoot(data, value);
+    RootedValue otherRoot(data, other.value);
+    MOZ_ASSERT(SameValue(nullptr, valueRoot, otherRoot, &same));
     MOZ_ASSERT(same == b);
 #endif
     return b;
 }
 
 HashableValue
 HashableValue::mark(JSTracer *trc) const
 {
--- a/js/src/jsapi-tests/testSameValue.cpp
+++ b/js/src/jsapi-tests/testSameValue.cpp
@@ -12,16 +12,16 @@ BEGIN_TEST(testSameValue)
 
     /*
      * NB: passing a double that fits in an integer jsval is API misuse.  As a
      * matter of defense in depth, however, JS_SameValue should return the
      * correct result comparing a positive-zero double to a negative-zero
      * double, and this is believed to be the only way to make such a
      * comparison possible.
      */
-    jsval v1 = DOUBLE_TO_JSVAL(0.0);
-    jsval v2 = DOUBLE_TO_JSVAL(-0.0);
+    JS::RootedValue v1(cx, JS::DoubleValue(0.0));
+    JS::RootedValue v2(cx, JS::DoubleValue(-0.0));
     bool same;
     CHECK(JS_SameValue(cx, v1, v2, &same));
     CHECK(!same);
     return true;
 }
 END_TEST(testSameValue)
--- a/js/src/jsapi-tests/testValueABI.cpp
+++ b/js/src/jsapi-tests/testValueABI.cpp
@@ -25,21 +25,22 @@ C_GetEmptyStringValue(JSContext *cx);
 extern size_t
 C_jsvalAlignmentTest();
 
 }
 
 BEGIN_TEST(testValueABI_retparam)
 {
     JS::RootedObject obj(cx, JS::CurrentGlobalOrNull(cx));
-    jsval v = OBJECT_TO_JSVAL(obj);
+    RootedValue v(cx, ObjectValue(*obj));
     obj = nullptr;
     CHECK(C_ValueToObject(cx, v, obj.address()));
     bool equal;
-    CHECK(JS_StrictlyEqual(cx, v, OBJECT_TO_JSVAL(obj), &equal));
+    RootedValue v2(cx, ObjectValue(*obj));
+    CHECK(JS_StrictlyEqual(cx, v, v2, &equal));
     CHECK(equal);
 
     v = C_GetEmptyStringValue(cx);
     CHECK(v.isString());
 
     return true;
 }
 END_TEST(testValueABI_retparam)
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -280,49 +280,43 @@ JS_TypeOfValue(JSContext *cx, HandleValu
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value);
     return TypeOfValue(value);
 }
 
 JS_PUBLIC_API(bool)
-JS_StrictlyEqual(JSContext *cx, jsval value1, jsval value2, bool *equal)
+JS_StrictlyEqual(JSContext *cx, HandleValue value1, HandleValue value2, bool *equal)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value1, value2);
-    bool eq;
-    if (!StrictlyEqual(cx, value1, value2, &eq))
-        return false;
-    *equal = eq;
-    return true;
+    MOZ_ASSERT(equal);
+    return StrictlyEqual(cx, value1, value2, equal);
 }
 
 JS_PUBLIC_API(bool)
 JS_LooselyEqual(JSContext *cx, HandleValue value1, HandleValue value2, bool *equal)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value1, value2);
     MOZ_ASSERT(equal);
     return LooselyEqual(cx, value1, value2, equal);
 }
 
 JS_PUBLIC_API(bool)
-JS_SameValue(JSContext *cx, jsval value1, jsval value2, bool *same)
+JS_SameValue(JSContext *cx, HandleValue value1, HandleValue value2, bool *same)
 {
     AssertHeapIsIdle(cx);
     CHECK_REQUEST(cx);
     assertSameCompartment(cx, value1, value2);
-    bool s;
-    if (!SameValue(cx, value1, value2, &s))
-        return false;
-    *same = s;
-    return true;
+    MOZ_ASSERT(same);
+    return SameValue(cx, value1, value2, same);
 }
 
 JS_PUBLIC_API(bool)
 JS_IsBuiltinEvalFunction(JSFunction *fun)
 {
     return IsAnyBuiltinEval(fun);
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -1052,23 +1052,23 @@ JS_ValueToSource(JSContext *cx, JS::Hand
 
 extern JS_PUBLIC_API(bool)
 JS_DoubleIsInt32(double d, int32_t *ip);
 
 extern JS_PUBLIC_API(JSType)
 JS_TypeOfValue(JSContext *cx, JS::Handle<JS::Value> v);
 
 extern JS_PUBLIC_API(bool)
-JS_StrictlyEqual(JSContext *cx, jsval v1, jsval v2, bool *equal);
+JS_StrictlyEqual(JSContext *cx, JS::Handle<JS::Value> v1, JS::Handle<JS::Value> v2, bool *equal);
 
 extern JS_PUBLIC_API(bool)
 JS_LooselyEqual(JSContext *cx, JS::Handle<JS::Value> v1, JS::Handle<JS::Value> v2, bool *equal);
 
 extern JS_PUBLIC_API(bool)
-JS_SameValue(JSContext *cx, jsval v1, jsval v2, bool *same);
+JS_SameValue(JSContext *cx, JS::Handle<JS::Value> v1, JS::Handle<JS::Value> v2, bool *same);
 
 /* True iff fun is the global eval function. */
 extern JS_PUBLIC_API(bool)
 JS_IsBuiltinEvalFunction(JSFunction *fun);
 
 /* True iff fun is the Function constructor. */
 extern JS_PUBLIC_API(bool)
 JS_IsBuiltinFunctionConstructor(JSFunction *fun);
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -529,26 +529,28 @@ DefinePropertyOnObject(JSContext *cx, Ha
     }
 
     do {
         if (desc.isAccessorDescriptor()) {
             if (!shapeAccessorDescriptor)
                 break;
 
             if (desc.hasGet()) {
+                RootedValue getter(cx, shape->getterOrUndefined());
                 bool same;
-                if (!SameValue(cx, desc.getterValue(), shape->getterOrUndefined(), &same))
+                if (!SameValue(cx, desc.getterValue(), getter, &same))
                     return false;
                 if (!same)
                     break;
             }
 
             if (desc.hasSet()) {
+                RootedValue setter(cx, shape->setterOrUndefined());
                 bool same;
-                if (!SameValue(cx, desc.setterValue(), shape->setterOrUndefined(), &same))
+                if (!SameValue(cx, desc.setterValue(), setter, &same))
                     return false;
                 if (!same)
                     break;
             }
         } else {
             /*
              * Determine the current value of the property once, if the current
              * value might actually need to be used or preserved later.  NB: we
@@ -664,26 +666,28 @@ DefinePropertyOnObject(JSContext *cx, Ha
         }
 
         callDelProperty = !shapeHasDefaultGetter || !shapeHasDefaultSetter;
     } else {
         /* 8.12.9 step 11. */
         MOZ_ASSERT(desc.isAccessorDescriptor() && shape->isAccessorDescriptor());
         if (!shape->configurable()) {
             if (desc.hasSet()) {
+                RootedValue setter(cx, shape->setterOrUndefined());
                 bool same;
-                if (!SameValue(cx, desc.setterValue(), shape->setterOrUndefined(), &same))
+                if (!SameValue(cx, desc.setterValue(), setter, &same))
                     return false;
                 if (!same)
                     return Reject(cx, JSMSG_CANT_REDEFINE_PROP, throwError, id, rval);
             }
 
             if (desc.hasGet()) {
+                RootedValue getter(cx, shape->getterOrUndefined());
                 bool same;
-                if (!SameValue(cx, desc.getterValue(), shape->getterOrUndefined(), &same))
+                if (!SameValue(cx, desc.getterValue(), getter, &same))
                     return false;
                 if (!same)
                     return Reject(cx, JSMSG_CANT_REDEFINE_PROP, throwError, id, rval);
             }
         }
     }
 
     /* 8.12.9 step 12. */
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -697,37 +697,37 @@ js::HasInstance(JSContext *cx, HandleObj
 
     RootedValue val(cx, ObjectValue(*obj));
     js_ReportValueError(cx, JSMSG_BAD_INSTANCEOF_RHS,
                         JSDVG_SEARCH_STACK, val, NullPtr());
     return false;
 }
 
 static inline bool
-EqualGivenSameType(JSContext *cx, const Value &lval, const Value &rval, bool *equal)
+EqualGivenSameType(JSContext *cx, HandleValue lval, HandleValue rval, bool *equal)
 {
     MOZ_ASSERT(SameType(lval, rval));
 
     if (lval.isString())
         return EqualStrings(cx, lval.toString(), rval.toString(), equal);
     if (lval.isDouble()) {
         *equal = (lval.toDouble() == rval.toDouble());
         return true;
     }
     if (lval.isGCThing()) {  // objects or symbols
         *equal = (lval.toGCThing() == rval.toGCThing());
         return true;
     }
-    *equal = lval.payloadAsRawUint32() == rval.payloadAsRawUint32();
-    MOZ_ASSERT_IF(lval.isUndefined(), *equal);
+    *equal = lval.get().payloadAsRawUint32() == rval.get().payloadAsRawUint32();
+    MOZ_ASSERT_IF(lval.isUndefined() || lval.isNull(), *equal);
     return true;
 }
 
 static inline bool
-LooselyEqualBooleanAndOther(JSContext *cx, const Value &lval, const Value &rval, bool *result)
+LooselyEqualBooleanAndOther(JSContext *cx, HandleValue lval, HandleValue rval, bool *result)
 {
     MOZ_ASSERT(!rval.isBoolean());
     RootedValue lvalue(cx, Int32Value(lval.toBoolean() ? 1 : 0));
 
     // The tail-call would end up in Step 3.
     if (rval.isNumber()) {
         *result = (lvalue.toNumber() == rval.toNumber());
         return true;
@@ -741,17 +741,17 @@ LooselyEqualBooleanAndOther(JSContext *c
         return true;
     }
 
     return LooselyEqual(cx, lvalue, rval, result);
 }
 
 // ES6 draft rev32 7.2.12 Abstract Equality Comparison
 bool
-js::LooselyEqual(JSContext *cx, const Value &lval, const Value &rval, bool *result)
+js::LooselyEqual(JSContext *cx, HandleValue lval, HandleValue rval, bool *result)
 {
     // Step 3.
     if (SameType(lval, rval))
         return EqualGivenSameType(cx, lval, rval, result);
 
     // Handle int32 x double.
     if (lval.isNumber() && rval.isNumber()) {
         *result = (lval.toNumber() == rval.toNumber());
@@ -816,19 +816,18 @@ js::LooselyEqual(JSContext *cx, const Va
     }
 
     // Step 12.
     *result = false;
     return true;
 }
 
 bool
-js::StrictlyEqual(JSContext *cx, const Value &lref, const Value &rref, bool *equal)
+js::StrictlyEqual(JSContext *cx, HandleValue lval, HandleValue rval, bool *equal)
 {
-    Value lval = lref, rval = rref;
     if (SameType(lval, rval))
         return EqualGivenSameType(cx, lval, rval, equal);
 
     if (lval.isNumber() && rval.isNumber()) {
         *equal = (lval.toNumber() == rval.toNumber());
         return true;
     }
 
@@ -844,17 +843,17 @@ IsNegativeZero(const Value &v)
 
 static inline bool
 IsNaN(const Value &v)
 {
     return v.isDouble() && mozilla::IsNaN(v.toDouble());
 }
 
 bool
-js::SameValue(JSContext *cx, const Value &v1, const Value &v2, bool *same)
+js::SameValue(JSContext *cx, HandleValue v1, HandleValue v2, bool *same)
 {
     if (IsNegativeZero(v1)) {
         *same = IsNegativeZero(v2);
         return true;
     }
     if (IsNegativeZero(v2)) {
         *same = false;
         return true;
@@ -2086,20 +2085,20 @@ END_CASE(JSOP_EQ)
 
 CASE(JSOP_NE)
     if (!LooseEqualityOp<false>(cx, REGS))
         goto error;
 END_CASE(JSOP_NE)
 
 #define STRICT_EQUALITY_OP(OP, COND)                                          \
     JS_BEGIN_MACRO                                                            \
-        const Value &rref = REGS.sp[-1];                                      \
-        const Value &lref = REGS.sp[-2];                                      \
+        HandleValue lval = REGS.stackHandleAt(-2);                            \
+        HandleValue rval = REGS.stackHandleAt(-1);                            \
         bool equal;                                                           \
-        if (!StrictlyEqual(cx, lref, rref, &equal))                           \
+        if (!StrictlyEqual(cx, lval, rval, &equal))                           \
             goto error;                                                       \
         (COND) = equal OP true;                                               \
         REGS.sp--;                                                            \
     JS_END_MACRO
 
 CASE(JSOP_STRICTEQ)
 {
     bool cond;
--- a/js/src/vm/Interpreter.h
+++ b/js/src/vm/Interpreter.h
@@ -216,24 +216,24 @@ class InvokeState : public RunState
         args_.rval().set(v);
     }
 };
 
 extern bool
 RunScript(JSContext *cx, RunState &state);
 
 extern bool
-StrictlyEqual(JSContext *cx, const Value &lval, const Value &rval, bool *equal);
+StrictlyEqual(JSContext *cx, HandleValue lval, HandleValue rval, bool *equal);
 
 extern bool
-LooselyEqual(JSContext *cx, const Value &lval, const Value &rval, bool *equal);
+LooselyEqual(JSContext *cx, HandleValue lval, HandleValue rval, bool *equal);
 
 /* === except that NaN is the same as NaN and -0 is not the same as +0. */
 extern bool
-SameValue(JSContext *cx, const Value &v1, const Value &v2, bool *same);
+SameValue(JSContext *cx, HandleValue v1, HandleValue v2, bool *same);
 
 extern JSType
 TypeOfObject(JSObject *obj);
 
 extern JSType
 TypeOfValue(const Value &v);
 
 extern bool