Bug 1092537 - Handle optional iterable argument in WeakMap constructor. r=evilpie
authorTooru Fujisawa <arai_a@mac.com>
Tue, 04 Nov 2014 00:23:38 +0900
changeset 231900 dae8ccc52200b44ba0bf0cb27d1cfc5cab3d0be5
parent 231899 c01469b24e22d378ef1bff3fd9610c18ce79bb3a
child 231901 bfedb105396ee4713a7d0a7b1b4d0dc85d6f89dc
push id7326
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:58:42 +0000
treeherdermozilla-aurora@d3a3b2a0f2f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1092537
milestone36.0a1
Bug 1092537 - Handle optional iterable argument in WeakMap constructor. r=evilpie
js/src/builtin/MapObject.cpp
js/src/jit-test/tests/collections/WeakMap-constructor-1.js
js/src/jit-test/tests/collections/WeakMap-constructor-2.js
js/src/jit-test/tests/collections/WeakMap-constructor-3.js
js/src/jit-test/tests/collections/WeakMap-constructor-4.js
js/src/jit-test/tests/collections/WeakMap-constructor-5.js
js/src/jit-test/tests/collections/WeakMap-constructor-arraylike-exception.js
js/src/jit-test/tests/collections/WeakMap-constructor-duplicates.js
js/src/jit-test/tests/collections/WeakMap-constructor-generator-1.js
js/src/jit-test/tests/collections/WeakMap-constructor-generator-2.js
js/src/jit-test/tests/collections/WeakMap-constructor-generator-3.js
js/src/jit-test/tests/collections/WeakMap-constructor-generator-exception.js
js/src/jit-test/tests/collections/WeakMap-constructor-iterable.js
js/src/jit-test/tests/collections/WeakMap-constructor-non-iterable.js
js/src/jit-test/tests/collections/WeakMap-constructor-nonnull.js
js/src/js.msg
js/src/jsweakmap.cpp
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -1245,17 +1245,18 @@ MapObject::construct(JSContext *cx, unsi
         ValueMap *map = obj->getData();
         while (true) {
             bool done;
             if (!iter.next(&pairVal, &done))
                 return false;
             if (done)
                 break;
             if (!pairVal.isObject()) {
-                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_INVALID_MAP_ITERABLE);
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr,
+                                     JSMSG_INVALID_MAP_ITERABLE, "Map");
                 return false;
             }
 
             pairObj = &pairVal.toObject();
             if (!pairObj)
                 return false;
 
             RootedValue key(cx);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-1.js
@@ -0,0 +1,15 @@
+// The WeakMap constructor creates an empty WeakMap by default.
+
+load(libdir + "asserts.js");
+
+new WeakMap();
+new WeakMap(undefined);
+
+// FIXME: bug 1092538
+// new WeakMap(null);
+
+// FIXME: bug 1062075
+// assertThrowsInstanceOf(() => WeakMap(), TypeError);
+// assertThrowsInstanceOf(() => WeakMap(undefined), TypeError);
+// WeakMap(null) throws TypeError, but it's because of bug 1092538.
+// assertThrowsInstanceOf(() => WeakMap(null), TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-2.js
@@ -0,0 +1,37 @@
+// The WeakMap constructor can take an argument that is an array of pairs.
+
+var k1 = {};
+var v1 = 42;
+var k2 = {};
+var v2 = 43;
+var k3 = {};
+
+var arr = [[k1, v1], [k2, v2]];
+
+var m = new WeakMap(arr);
+
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), undefined);
+
+var arraylike1 = {
+  0: k1,
+  1: v1
+};
+var arraylike2 = {
+  0: k2,
+  1: v2
+};
+arr = [arraylike1, arraylike2];
+
+m = new WeakMap(arr);
+
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), undefined);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-3.js
@@ -0,0 +1,35 @@
+// WeakMap can take an argument that is an array of singleton arrays.
+
+var k1 = {};
+var k2 = {};
+var k3 = {};
+
+var arr = [[k1], [k2]];
+
+var m = new WeakMap(arr);
+
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), undefined);
+assertEq(m.get(k2), undefined);
+assertEq(m.get(k3), undefined);
+
+var arraylike1 = {
+  0: k1,
+  1: undefined
+};
+var arraylike2 = {
+  0: k2,
+};
+
+arr = [arraylike1, arraylike2];
+
+m = new WeakMap(arr);
+
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), undefined);
+assertEq(m.get(k2), undefined);
+assertEq(m.get(k3), undefined);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-4.js
@@ -0,0 +1,8 @@
+// WeakMap(x) throws if x is not iterable (unless x is undefined).
+
+load(libdir + "asserts.js");
+// FIXME: bug 1092538
+// `new WeapMap(null)` should not throw.
+var nonIterables = [null, true, 1, -0, 3.14, NaN, {}, Math, this];
+for (let k of nonIterables)
+    assertThrowsInstanceOf(function () { new WeakMap(k); }, TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-5.js
@@ -0,0 +1,23 @@
+// WeakMap(arr) throws if arr contains holes (or undefined values).
+
+load(libdir + "asserts.js");
+
+var k1 = {};
+var v1 = 42;
+var k2 = {};
+var v2 = 43;
+var k3 = {};
+var v3 = {};
+
+assertThrowsInstanceOf(function () { new WeakMap([undefined]); }, TypeError);
+assertThrowsInstanceOf(function () { new WeakMap([null]); }, TypeError);
+assertThrowsInstanceOf(function () { new WeakMap([[k1, v1], [k2, v2], , [k3, k3]]); }, TypeError);
+assertThrowsInstanceOf(function () { new WeakMap([[k1, v1], [k2, v2], ,]); }, TypeError);
+
+// WeakMap(iterable) throws if iterable doesn't have array-like objects
+
+assertThrowsInstanceOf(function () { new WeakMap([1, 2, 3]); }, TypeError);
+assertThrowsInstanceOf(function () {
+  let s = new Set([1, 2, "abc"]);
+  new WeakMap(s);
+}, TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-arraylike-exception.js
@@ -0,0 +1,23 @@
+// WeakMap constructor should propagate exception while getting key and value.
+
+load(libdir + "asserts.js");
+
+var k1 = {};
+var v1 = 42;
+
+var error_thrower_0 = {
+  get 0() {
+    throw new Error;
+  },
+  1: v1
+};
+assertThrowsInstanceOf(() => new WeakMap([error_thrower_0]), Error);
+
+var error_thrower_1 = {
+  0: k1,
+  get 1() {
+    throw new Error;
+  }
+};
+assertThrowsInstanceOf(() => new WeakMap([error_thrower_1]), Error);
+
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-duplicates.js
@@ -0,0 +1,27 @@
+// When the argument to WeakMap contains a key multiple times, the last value
+// is retained.
+
+var k1 = {};
+var v1 = 42;
+var k2 = {};
+var v2 = 43;
+var k3 = {};
+var v3 = 44;
+var k4 = {};
+
+var wrong1 = 8;
+var wrong2 = 9;
+var wrong3 = 10;
+
+var arr = [[k1, wrong1], [k2, v2], [k3, wrong2], [k1, wrong3], [k3, v3], [k1, v1]];
+
+var m = new WeakMap(arr);
+
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), true);
+assertEq(m.has(k4), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), v3);
+assertEq(m.get(k4), undefined);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-generator-1.js
@@ -0,0 +1,43 @@
+// The argument to WeakMap can be a generator.
+
+var k1 = {};
+var v1 = 42;
+var k2 = {};
+var v2 = 43;
+var k3 = {};
+
+var done = false;
+
+function data() {
+  yield [k1, v1];
+  yield [k2, v2];
+  done = true;
+};
+
+var m = new WeakMap(data());
+
+assertEq(done, true);  // the constructor consumes the argument
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), undefined);
+
+done = false;
+
+function* data2() {
+  yield [k1, v1];
+  yield [k2, v2];
+  done = true;
+};
+
+m = new WeakMap(data2());
+
+assertEq(done, true);  // the constructor consumes the argument
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), undefined);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-generator-2.js
@@ -0,0 +1,19 @@
+// The argument to WeakMap can be a generator-expression.
+
+var k1 = {};
+var k2 = {};
+var k3 = {};
+var k4 = {};
+
+var valueToKey = {
+  1: k1,
+  2: k2,
+  "green": k3,
+  "red": k4
+};
+
+var arr = [1, 2, "green", "red"];
+var m = new WeakMap([valueToKey[v], v] for (v of arr));
+
+for (var i = 0; i < 4; i++)
+  assertEq(m.get(valueToKey[arr[i]]), arr[i]);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-generator-3.js
@@ -0,0 +1,13 @@
+// The argument to WeakMap may be a generator-iterator that produces no values.
+
+new WeakMap(x for (x of []));
+
+function none() {
+    if (0) yield 0;
+}
+new WeakMap(none());
+
+function* none2() {
+    if (0) yield 0;
+}
+new WeakMap(none2());
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-generator-exception.js
@@ -0,0 +1,22 @@
+// Iterating over the argument to WeakMap can throw. The exception is
+// propagated.
+
+load(libdir + "asserts.js");
+
+function data() {
+    yield [{}, "XR22/Z"];
+    yield [{}, "23D-BN"];
+    throw "oops";
+}
+
+var it = data();
+assertThrowsValue(() => new WeakMap(it), "oops");
+
+function* data2() {
+    yield [{}, "XR22/Z"];
+    yield [{}, "23D-BN"];
+    throw "oops";
+}
+
+var it2 = data2();
+assertThrowsValue(() => new WeakMap(it2), "oops");
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-iterable.js
@@ -0,0 +1,28 @@
+// The argument to WeakMap can be a iterable object.
+
+load(libdir + "iteration.js");
+
+var k1 = {};
+var v1 = 42;
+var k2 = {};
+var v2 = 43;
+var k3 = {};
+
+var done = false;
+
+var iterable = {};
+iterable[std_iterator] = function*() {
+  yield [k1, v1];
+  yield [k2, v2];
+  done = true;
+};
+var m = new WeakMap(iterable);
+
+assertEq(done, true);  // the constructor consumes the argument
+assertEq(m.has(k1), true);
+assertEq(m.has(k2), true);
+assertEq(m.has(k3), false);
+assertEq(m.get(k1), v1);
+assertEq(m.get(k2), v2);
+assertEq(m.get(k3), undefined);
+
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-non-iterable.js
@@ -0,0 +1,13 @@
+// WeakMap should throw if argument is not iterable object.
+
+load(libdir + "asserts.js");
+load(libdir + "iteration.js");
+
+var non_iterable1 = {};
+non_iterable1[std_iterator] = {};
+assertThrowsInstanceOf(() => new WeakMap(non_iterable1), TypeError);
+
+var non_iterable2 = {};
+non_iterable2[std_iterator] = function() {
+};
+assertThrowsInstanceOf(() => new WeakMap(non_iterable2), TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/WeakMap-constructor-nonnull.js
@@ -0,0 +1,11 @@
+// WeakMap constructor should throw when key is nonnull.
+
+load(libdir + "asserts.js");
+
+var v1 = 42;
+
+var primitive = 10;
+assertThrowsInstanceOf(() => new WeakMap([[primitive, v1]]), TypeError);
+
+var empty_array = [];
+assertThrowsInstanceOf(() => new WeakMap([empty_array]), TypeError);
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -60,17 +60,17 @@ MSG_DEF(JSMSG_ARG_INDEX_OUT_OF_RANGE,  1
 MSG_DEF(JSMSG_SPREAD_TOO_LARGE,        0, JSEXN_RANGEERR, "array too large due to spread operand(s)")
 MSG_DEF(JSMSG_BAD_WEAKMAP_KEY,         0, JSEXN_TYPEERR, "cannot use the given object as a weak map key")
 MSG_DEF(JSMSG_BAD_GETTER_OR_SETTER,    1, JSEXN_TYPEERR, "invalid {0} usage")
 MSG_DEF(JSMSG_BAD_ARRAY_LENGTH,        0, JSEXN_RANGEERR, "invalid array length")
 MSG_DEF(JSMSG_REDECLARED_VAR,          2, JSEXN_TYPEERR, "redeclaration of {0} {1}")
 MSG_DEF(JSMSG_UNDECLARED_VAR,          1, JSEXN_REFERENCEERR, "assignment to undeclared variable {0}")
 MSG_DEF(JSMSG_GETTER_ONLY,             0, JSEXN_TYPEERR, "setting a property that has only a getter")
 MSG_DEF(JSMSG_UNDEFINED_PROP,          1, JSEXN_REFERENCEERR, "reference to undefined property {0}")
-MSG_DEF(JSMSG_INVALID_MAP_ITERABLE,    0, JSEXN_TYPEERR, "iterable for map should have array-like objects")
+MSG_DEF(JSMSG_INVALID_MAP_ITERABLE,    1, JSEXN_TYPEERR, "iterable for {0} should have array-like objects")
 MSG_DEF(JSMSG_NESTING_GENERATOR,       0, JSEXN_TYPEERR, "already executing generator")
 MSG_DEF(JSMSG_INCOMPATIBLE_METHOD,     3, JSEXN_TYPEERR, "{0} {1} called on incompatible {2}")
 MSG_DEF(JSMSG_OBJECT_WATCH_DEPRECATED, 0, JSEXN_NONE, "Object.prototype.watch and unwatch are very slow, non-standard, and deprecated; use a getter/setter instead")
 MSG_DEF(JSMSG_TYPE_ERR_BAD_ARGS,       0, JSEXN_TYPEERR, "invalid arguments")
 MSG_DEF(JSMSG_BAD_SURROGATE_CHAR,      1, JSEXN_TYPEERR, "bad surrogate character {0}")
 MSG_DEF(JSMSG_UTF8_CHAR_TOO_LARGE,     1, JSEXN_TYPEERR, "UTF-8 character {0} too large")
 MSG_DEF(JSMSG_MALFORMED_UTF8_CHAR,     1, JSEXN_TYPEERR, "malformed UTF-8 character sequence at offset {0}")
 MSG_DEF(JSMSG_WRONG_CONSTRUCTOR,       1, JSEXN_TYPEERR, "wrong constructor called for {0}")
--- a/js/src/jsweakmap.cpp
+++ b/js/src/jsweakmap.cpp
@@ -517,20 +517,71 @@ JS::SetWeakMapEntry(JSContext *cx, Handl
     Rooted<WeakMapObject*> rootedMap(cx, &mapObj->as<WeakMapObject>());
     return SetWeakMapEntryInternal(cx, rootedMap, key, val);
 }
 
 static bool
 WeakMap_construct(JSContext *cx, unsigned argc, Value *vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
-    JSObject *obj = NewBuiltinClassInstance(cx, &WeakMapObject::class_);
+    RootedObject obj(cx, NewBuiltinClassInstance(cx, &WeakMapObject::class_));
     if (!obj)
         return false;
 
+    // ES6 23.3.1.1 steps 5-6, 11.
+    if (args.hasDefined(0)) {
+        // Steps 7d-e.
+        JS::ForOfIterator iter(cx);
+        if (!iter.init(args[0]))
+            return false;
+
+        RootedValue pairVal(cx);
+        RootedObject pairObject(cx);
+        RootedValue keyVal(cx);
+        RootedObject keyObject(cx);
+        RootedValue val(cx);
+        while (true) {
+            // Steps 12a-e.
+            bool done;
+            if (!iter.next(&pairVal, &done))
+                return false;
+            if (done)
+                break;
+
+            // Step 12f.
+            if (!pairVal.isObject()) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr,
+                                     JSMSG_INVALID_MAP_ITERABLE, "WeakMap");
+                return false;
+            }
+
+            pairObject = &pairVal.toObject();
+            if (!pairObject)
+                return false;
+
+            // Steps 12g-h.
+            if (!JSObject::getElement(cx, pairObject, pairObject, 0, &keyVal))
+                return false;
+
+            // Steps 12i-j.
+            if (!JSObject::getElement(cx, pairObject, pairObject, 1, &val))
+                return false;
+
+            // Steps 12k-l.
+            if (keyVal.isPrimitive()) {
+                JS_ReportErrorNumber(cx, js_GetErrorMessage, nullptr, JSMSG_NOT_NONNULL_OBJECT);
+                return false;
+            }
+
+            keyObject = &keyVal.toObject();
+            if (!SetWeakMapEntry(cx, obj, keyObject, val))
+                return false;
+        }
+    }
+
     args.rval().setObject(*obj);
     return true;
 }
 
 const Class WeakMapObject::class_ = {
     "WeakMap",
     JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS |
     JSCLASS_HAS_CACHED_PROTO(JSProto_WeakMap),