Bug 515285 - Implement JS_SameValue, and make assertEq use it rather than JS_StrictlyEqual. r=jorendorff

js/src/jsapi-tests/Makefile.in

@@ -51,6 +51,7 @@ CPPSRCS = \
   testPropCache.cpp \
   testXDR.cpp \
   testIntString.cpp \
+  testSameValue.cpp \
   $(NULL)
 
 DEFINES         += -DEXPORT_JS_API

js/src/jsapi-tests/selfTest.cpp

@@ -12,15 +12,6 @@ BEGIN_TEST(selfTest_NaNsAreSame)
 }
 END_TEST(selfTest_NaNsAreSame)
 
-BEGIN_TEST(selfTest_negativeZeroIsNotTheSameAsZero)
-{
-    jsvalRoot negativeZero(cx);
-    EVAL("-0", negativeZero.addr());
-    CHECK(!sameValue(negativeZero, JSVAL_ZERO));
-    return true;
-}
-END_TEST(selfTest_negativeZeroIsNotTheSameAsZero)
-
 BEGIN_TEST(selfTest_globalHasNoParent)
 {
     CHECK(JS_GetParent(cx, global) == NULL);

js/src/jsapi-tests/testSameValue.cpp

@@ -0,0 +1,21 @@
+#include "tests.h"
+
+BEGIN_TEST(testSameValue)
+{
+    jsvalRoot v1(cx);
+    jsvalRoot v2(cx);
+
+    /*
+     * 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.
+     */
+    CHECK(JS_NewDoubleValue(cx, 0.0, v1.addr()));
+    CHECK(JSVAL_IS_DOUBLE(v1));
+    CHECK(JS_NewNumberValue(cx, -0.0, v2.addr()));
+    CHECK(!JS_SameValue(cx, v1, v2));
+    return true;
+}
+END_TEST(testSameValue)

js/src/jsapi-tests/tests.h

@@ -1,5 +1,5 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*-
- * vim: set ts=8 sw=4 et tw=78:
+ * vim: set ts=8 sw=4 et tw=99:
  *
  * ***** BEGIN LICENSE BLOCK *****
  * Version: MPL 1.1/GPL 2.0/LGPL 2.1
@@ -118,34 +118,6 @@ public:
     virtual const char * name() = 0;
     virtual bool run() = 0;
 
-    bool isNegativeZero(jsval v) {
-        if (!JSVAL_IS_DOUBLE(v))
-            return false;
-        union {
-            uint64   u64;
-            jsdouble d;
-        } pun;
-        pun.d = *JSVAL_TO_DOUBLE(v);
-        return pun.d == jsdouble(-0.0) && pun.u64 != uint64(0);
-    }
-
-    bool isNaN(jsval v) {
-        if (!JSVAL_IS_DOUBLE(v))
-            return false;
-        jsdouble d = *JSVAL_TO_DOUBLE(v);
-        return d != d;
-    }
-
-    bool sameValue(jsval v1, jsval v2) {
-        if ((isNegativeZero(v1) && v2 == JSVAL_ZERO) ||
-            (isNegativeZero(v2) && v1 == JSVAL_ZERO)) {
-            return false;
-        }
-        if (isNaN(v1) && isNaN(v2))
-            return true;
-        return JS_StrictlyEqual(cx, v1, v2);
-    }
-
 #define EXEC(s) do { if (!exec(s, __FILE__, __LINE__)) return false; } while (false)
 
     bool exec(const char *bytes, const char *filename, int lineno) {
@@ -179,9 +151,9 @@ public:
     bool checkSame(jsval actual, jsval expected,
                    const char *actualExpr, const char *expectedExpr,
                    const char *filename, int lineno) {
-        return sameValue(actual, expected) ||
-            fail(std::string("CHECK_SAME failed: expected sameValue(") +
-                 actualExpr + ", " + expectedExpr + "), got !sameValue(" +
+        return JS_SameValue(cx, actual, expected) ||
+            fail(std::string("CHECK_SAME failed: expected JS_SameValue(cx, ") +
+                 actualExpr + ", " + expectedExpr + "), got !JS_SameValue(cx, " +
                  toSource(actual) + ", " + toSource(expected) + ")", filename, lineno);
     }
 

js/src/jsapi.cpp

@@ -711,6 +711,12 @@ JS_StrictlyEqual(JSContext *cx, jsval v1, jsval v2)
     return js_StrictlyEqual(cx, v1, v2);
 }
 
+JS_PUBLIC_API(JSBool)
+JS_SameValue(JSContext *cx, jsval v1, jsval v2)
+{
+    return js_SameValue(v1, v2, cx);
+}
+
 /************************************************************************/
 
 /*

js/src/jsapi.h

@@ -528,6 +528,9 @@ JS_GetTypeName(JSContext *cx, JSType type);
 extern JS_PUBLIC_API(JSBool)
 JS_StrictlyEqual(JSContext *cx, jsval v1, jsval v2);
 
+extern JS_PUBLIC_API(JSBool)
+JS_SameValue(JSContext *cx, jsval v1, jsval v2);
+
 /************************************************************************/
 
 /*

js/src/jsinterp.cpp

@@ -1824,6 +1824,30 @@ js_StrictlyEqual(JSContext *cx, jsval lval, jsval rval)
     return lval == rval;
 }
 
+static inline bool
+IsNegativeZero(jsval v)
+{
+    return JSVAL_IS_DOUBLE(v) && JSDOUBLE_IS_NEGZERO(*JSVAL_TO_DOUBLE(v));
+}
+
+static inline bool
+IsNaN(jsval v)
+{
+    return JSVAL_IS_DOUBLE(v) && JSDOUBLE_IS_NaN(*JSVAL_TO_DOUBLE(v));
+}
+
+JSBool
+js_SameValue(jsval v1, jsval v2, JSContext *cx)
+{
+    if (IsNegativeZero(v1))
+        return IsNegativeZero(v2);
+    if (IsNegativeZero(v2))
+        return JS_FALSE;
+    if (IsNaN(v1) && IsNaN(v2))
+        return JS_TRUE;
+    return js_StrictlyEqual(cx, v1, v2);
+}
+
 JS_REQUIRES_STACK JSBool
 js_InvokeConstructor(JSContext *cx, uintN argc, JSBool clampReturn, jsval *vp)
 {

js/src/jsinterp.h

@@ -554,6 +554,10 @@ js_CheckRedeclaration(JSContext *cx, JSObject *obj, jsid id, uintN attrs,
 extern JSBool
 js_StrictlyEqual(JSContext *cx, jsval lval, jsval rval);
 
+/* === except that NaN is the same as NaN and -0 is not the same as +0. */
+extern JSBool
+js_SameValue(jsval v1, jsval v2, JSContext *cx);
+
 extern JSBool
 js_InternNonIntElementId(JSContext *cx, JSObject *obj, jsval idval, jsid *idp);
 

js/src/jsshell.msg

@@ -52,3 +52,5 @@ MSG_DEF(JSSMSG_SCRIPTS_ONLY,             7, 0, JSEXN_NONE, "only works on script
 MSG_DEF(JSSMSG_NOT_ENOUGH_ARGS,          8, 1, JSEXN_NONE, "{0}: not enough arguments")
 MSG_DEF(JSSMSG_TOO_MANY_ARGS,            9, 1, JSEXN_NONE, "{0}: too many arguments")
 MSG_DEF(JSSMSG_ASSERT_EQ_FAILED,        10, 2, JSEXN_NONE, "Assertion failed: got {0}, expected {1}")
+MSG_DEF(JSSMSG_ASSERT_EQ_FAILED_MSG,    11, 3, JSEXN_NONE, "Assertion failed: got {0}, expected {1}: {2}")
+MSG_DEF(JSSMSG_INVALID_ARGS,            12, 1, JSEXN_NONE, "{0}: invalid arguments")

js/src/shell/js.cpp

@@ -1078,19 +1078,28 @@ ToSource(JSContext *cx, jsval *vp)
 static JSBool
 AssertEq(JSContext *cx, uintN argc, jsval *vp)
 {
-    if (argc != 2) {
+    if (!(argc == 2 || (argc == 3 && JSVAL_IS_STRING(JS_ARGV(cx, vp)[2])))) {
         JS_ReportErrorNumber(cx, my_GetErrorMessage, NULL,
-                             (argc > 2) ? JSSMSG_TOO_MANY_ARGS : JSSMSG_NOT_ENOUGH_ARGS,
+                             (argc < 2)
+                             ? JSSMSG_NOT_ENOUGH_ARGS
+                             : (argc == 3)
+                             ? JSSMSG_INVALID_ARGS
+                             : JSSMSG_TOO_MANY_ARGS,
                              "assertEq");
         return JS_FALSE;
     }
 
     jsval *argv = JS_ARGV(cx, vp);
-    if (!JS_StrictlyEqual(cx, argv[0], argv[1])) {
+    if (!JS_SameValue(cx, argv[0], argv[1])) {
         const char *actual = ToSource(cx, &argv[0]);
         const char *expected = ToSource(cx, &argv[1]);
-        JS_ReportErrorNumber(cx, my_GetErrorMessage, NULL, JSSMSG_ASSERT_EQ_FAILED,
-                             actual, expected);
+        if (argc == 2) {
+            JS_ReportErrorNumber(cx, my_GetErrorMessage, NULL, JSSMSG_ASSERT_EQ_FAILED,
+                                 actual, expected);
+        } else {
+            JS_ReportErrorNumber(cx, my_GetErrorMessage, NULL, JSSMSG_ASSERT_EQ_FAILED_MSG,
+                                 actual, expected, JS_GetStringBytes(JSVAL_TO_STRING(argv[2])));
+        }
         return JS_FALSE;
     }
     JS_SET_RVAL(cx, vp, JSVAL_VOID);
@@ -3725,8 +3734,9 @@ static const char *const shell_help_messages[] = {
 "print([exp ...])         Evaluate and print expressions",
 "help([name ...])         Display usage and help messages",
 "quit()                   Quit the shell",
-"assertEq(actual, expected)\n"
-"                         Throw if the two arguments are not ===",
+"assertEq(actual, expected[, msg])\n"
+"  Throw if the first two arguments are not the same (both +0 or both -0,\n"
+"  both NaN, or non-zero and ===)",
 "gc()                     Run the garbage collector",
 "gcparam(name, value)\n"
 "  Wrapper for JS_SetGCParameter. The name must be either 'maxBytes' or\n"