Bug 722260 - Properly handle NaN values as keys in Map objects. r=luke
authorJeff Walden <jwalden@mit.edu>
Tue, 31 Jan 2012 16:49:27 -0800
changeset 85984 19addd0f1a6d04d483d5d6df2064b7fcd8e41620
parent 85983 551dcf40a209ee3ce2e708643fe2cee98acbf315
child 85985 13304ad28754a4bd3f92dc3a42914ed4fb46ecd6
push id21982
push userbmo@edmorley.co.uk
push dateThu, 02 Feb 2012 10:23:53 +0000
treeherdermozilla-central@005980552224 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs722260
milestone13.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 722260 - Properly handle NaN values as keys in Map objects. r=luke
js/src/builtin/MapObject.cpp
js/src/tests/Makefile.in
js/src/tests/ecma_6/Map/NaN-as-key.js
js/src/tests/ecma_6/Map/browser.js
js/src/tests/ecma_6/Map/jstests.list
js/src/tests/ecma_6/Map/shell.js
js/src/tests/ecma_6/Set/NaN-as-key.js
js/src/tests/ecma_6/Set/browser.js
js/src/tests/ecma_6/Set/jstests.list
js/src/tests/ecma_6/Set/shell.js
js/src/tests/ecma_6/browser.js
js/src/tests/ecma_6/jstests.list
js/src/tests/ecma_6/shell.js
js/src/tests/jstests.list
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -85,24 +85,20 @@ HashableValue::setValue(JSContext *cx, c
             return false;
         value = StringValue(str);
     } else if (v.isDouble()) {
         jsdouble d = v.toDouble();
         int32_t i;
         if (JSDOUBLE_IS_INT32(d, &i)) {
             /* Normalize int32-valued doubles to int32 for faster hashing and testing. */
             value = Int32Value(i);
+        } else if (JSDOUBLE_IS_NaN(d)) {
+            /* NaNs with different bits must hash and test identically. */
+            value = DoubleValue(js_NaN);
         } else {
-#ifdef DEBUG
-            /* All NaN values are the same. The bit-pattern must reflect this. */
-            jsval_layout a, b;
-            a.asDouble = d;
-            b.asDouble = JS_CANONICALIZE_NAN(d);
-            JS_ASSERT(a.asBits == b.asBits);
-#endif
             value = v;
         }
     } else {
         value = v;
     }
 
     JS_ASSERT(value.isUndefined() || value.isNull() || value.isBoolean() ||
               value.isNumber() || value.isString() || value.isObject());
--- a/js/src/tests/Makefile.in
+++ b/js/src/tests/Makefile.in
@@ -54,16 +54,17 @@ TEST_FILES = \
   user.js \
   jstests.list \
   e4x/ \
   ecma/ \
   ecma_2/ \
   ecma_3/ \
   ecma_3_1/ \
   ecma_5/ \
+  ecma_6/ \
   js1_1/ \
   js1_2/ \
   js1_3/ \
   js1_4/ \
   js1_5/ \
   js1_6/ \
   js1_7/ \
   js1_8/ \
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Map/NaN-as-key.js
@@ -0,0 +1,55 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 722260;
+var summary = 'All NaNs must be treated as identical keys for Map';
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+/* Avoid constant-folding that would happen were |undefined| to be used. */
+var key = -/a/g.missingProperty;
+
+var m = new Map();
+m.set(key, 17);
+assertEq(m.get(key), 17);
+assertEq(m.get(-key), 17);
+assertEq(m.get(NaN), 17);
+
+m.delete(-key);
+assertEq(m.has(key), false);
+assertEq(m.has(-key), false);
+assertEq(m.has(NaN), false);
+
+m.set(-key, 17);
+assertEq(m.get(key), 17);
+assertEq(m.get(-key), 17);
+assertEq(m.get(NaN), 17);
+
+m.delete(NaN);
+assertEq(m.has(key), false);
+assertEq(m.has(-key), false);
+assertEq(m.has(NaN), false);
+
+m.set(NaN, 17);
+assertEq(m.get(key), 17);
+assertEq(m.get(-key), 17);
+assertEq(m.get(NaN), 17);
+
+m.delete(key);
+assertEq(m.has(key), false);
+assertEq(m.has(-key), false);
+assertEq(m.has(NaN), false);
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Map/jstests.list
@@ -0,0 +1,2 @@
+url-prefix ../../jsreftest.html?test=ecma_6/Map/
+script NaN-as-key.js
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Set/NaN-as-key.js
@@ -0,0 +1,56 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 722260;
+var summary = 'All NaNs must be treated as identical keys for Set';
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+/* Avoid constant-folding that would happen were |undefined| to be used. */
+var key = -/a/g.missingProperty;
+
+var s = new Set();
+s.add(key, 17);
+assertEq(s.has(key), true);
+assertEq(s.has(-key), true);
+assertEq(s.has(NaN), true);
+
+s.delete(-key);
+assertEq(s.has(key), false);
+assertEq(s.has(-key), false);
+assertEq(s.has(NaN), false);
+
+s.add(-key, 17);
+assertEq(s.has(key), true);
+assertEq(s.has(-key), true);
+assertEq(s.has(NaN), true);
+
+s.delete(NaN);
+assertEq(s.has(key), false);
+assertEq(s.has(-key), false);
+assertEq(s.has(NaN), false);
+
+s.add(NaN, 17);
+assertEq(s.has(key), true);
+assertEq(s.has(-key), true);
+assertEq(s.has(NaN), true);
+
+s.delete(key);
+assertEq(s.has(key), false);
+assertEq(s.has(-key), false);
+assertEq(s.has(NaN), false);
+
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Set/jstests.list
@@ -0,0 +1,2 @@
+url-prefix ../../jsreftest.html?test=ecma_6/Set/
+script NaN-as-key.js
new file mode 100644
new file mode 100644
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/jstests.list
@@ -0,0 +1,2 @@
+include Map/jstests.list
+include Set/jstests.list
new file mode 100644
--- a/js/src/tests/jstests.list
+++ b/js/src/tests/jstests.list
@@ -1,14 +1,17 @@
+# If you add a new test subdirectory here, you *must* add it to Makefile.in as
+# well, or tinderbox will go up in flames.  YOU HAVE BEEN WARNED.
 include e4x/jstests.list
 include ecma/jstests.list
 include ecma_2/jstests.list
 include ecma_3/jstests.list
 include ecma_3_1/jstests.list
 include ecma_5/jstests.list
+include ecma_6/jstests.list
 include js1_1/jstests.list
 include js1_2/jstests.list
 include js1_3/jstests.list
 include js1_4/jstests.list
 include js1_5/jstests.list
 include js1_6/jstests.list
 include js1_7/jstests.list
 include js1_8/jstests.list