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 215658 dae8ccc52200
parent 215657 c01469b24e22
child 215659 bfedb105396e
push id51815
push userevilpies@gmail.com
push dateThu, 13 Nov 2014 23:59:32 +0000
treeherdermozilla-inbound@dae8ccc52200 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1092537
milestone36.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 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),