Make Map and Set constructors take a single iterable argument. Bug 726223, r=luke.
authorJason Orendorff <jorendorff@mozilla.com>
Thu, 01 Mar 2012 09:01:46 -0600
changeset 88293 9b944a4b62309b34e0b451f1ca353de89be02aa3
parent 88292 3f51e7052b7d5ac73a20d8e7ac067ad4b2e397b1
child 88294 21e7e8e9d75ab2ccf5d3104d6bec760a378596cb
push id157
push userMs2ger@gmail.com
push dateWed, 07 Mar 2012 19:27:10 +0000
reviewersluke
bugs726223
milestone13.0a1
Make Map and Set constructors take a single iterable argument. Bug 726223, r=luke.
js/src/builtin/MapObject.cpp
js/src/jit-test/tests/collections/Map-constructor-1.js
js/src/jit-test/tests/collections/Map-constructor-2.js
js/src/jit-test/tests/collections/Map-constructor-3.js
js/src/jit-test/tests/collections/Map-constructor-4.js
js/src/jit-test/tests/collections/Map-constructor-5.js
js/src/jit-test/tests/collections/Map-constructor-duplicates.js
js/src/jit-test/tests/collections/Map-constructor-generator-1.js
js/src/jit-test/tests/collections/Map-constructor-generator-2.js
js/src/jit-test/tests/collections/Map-constructor-generator-3.js
js/src/jit-test/tests/collections/Map-constructor-generator-exception.js
js/src/jit-test/tests/collections/Map-size-1.js
js/src/jit-test/tests/collections/Map-size-2.js
js/src/jit-test/tests/collections/Map-size.js
js/src/jit-test/tests/collections/Map-surfaces-1.js
js/src/jit-test/tests/collections/Set-constructor-1.js
js/src/jit-test/tests/collections/Set-constructor-2.js
js/src/jit-test/tests/collections/Set-constructor-3.js
js/src/jit-test/tests/collections/Set-constructor-generator-1.js
js/src/jit-test/tests/collections/Set-constructor-generator-2.js
js/src/jit-test/tests/collections/Set-size-1.js
js/src/jit-test/tests/collections/Set-size-2.js
js/src/jit-test/tests/collections/Set-size.js
js/src/jit-test/tests/collections/Set-surfaces-1.js
js/src/jsiter.cpp
js/src/jsiter.h
--- a/js/src/builtin/MapObject.cpp
+++ b/js/src/builtin/MapObject.cpp
@@ -37,16 +37,17 @@
  * the terms of any one of the MPL, the GPL or the LGPL.
  *
  * ***** END LICENSE BLOCK ***** */
 
 #include "builtin/MapObject.h"
 
 #include "jscntxt.h"
 #include "jsgcmark.h"
+#include "jsiter.h"
 #include "jsobj.h"
 
 #include "vm/GlobalObject.h"
 #include "vm/MethodGuard.h"
 #include "vm/Stack.h"
 
 #include "jsobjinlines.h"
 
@@ -57,17 +58,17 @@ InitClass(JSContext *cx, GlobalObject *g
           JSFunctionSpec *methods)
 {
     JSObject *proto = global->createBlankPrototype(cx, clasp);
     if (!proto)
         return NULL;
     proto->setPrivate(NULL);
 
     JSAtom *atom = cx->runtime->atomState.classAtoms[key];
-    JSFunction *ctor = global->createConstructor(cx, construct, clasp, atom, 0);
+    JSFunction *ctor = global->createConstructor(cx, construct, clasp, atom, 1);
     if (!ctor ||
         !LinkConstructorAndPrototype(cx, ctor, proto) ||
         !DefinePropertiesAndBrand(cx, proto, NULL, methods) ||
         !DefineConstructorAndPrototype(cx, global, key, ctor, proto))
     {
         return NULL;
     }
     return proto;
@@ -197,33 +198,71 @@ MapObject::mark(JSTracer *trc, JSObject 
 void
 MapObject::finalize(JSContext *cx, JSObject *obj)
 {
     MapObject *mapobj = static_cast<MapObject *>(obj);
     if (ValueMap *map = mapobj->getData())
         cx->delete_(map);
 }
 
+class AddToMap {
+  private:
+    ValueMap *map;
+
+  public:
+    AddToMap(ValueMap *map) : map(map) {}
+
+    bool operator()(JSContext *cx, const Value &v) {
+        JSObject *pairobj = js_ValueToNonNullObject(cx, v);
+        if (!pairobj)
+            return false;
+
+        Value key;
+        if (!pairobj->getElement(cx, 0, &key))
+            return false;
+        HashableValue hkey;
+        if (!hkey.setValue(cx, key))
+            return false;
+
+        Value val;
+        if (!pairobj->getElement(cx, 1, &val))
+            return false;
+
+        if (!map->put(hkey, val)) {
+            js_ReportOutOfMemory(cx);
+            return false;
+        }
+        return true;
+    }
+};
+
 JSBool
 MapObject::construct(JSContext *cx, unsigned argc, Value *vp)
 {
     JSObject *obj = NewBuiltinClassInstance(cx, &class_);
     if (!obj)
         return false;
 
     ValueMap *map = cx->new_<ValueMap>(cx->runtime);
     if (!map)
         return false;
     if (!map->init()) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     obj->setPrivate(map);
 
-    CallArgsFromVp(argc, vp).rval().setObject(*obj);
+    CallArgs args = CallArgsFromVp(argc, vp);
+    Value arg = (argc > 0 ? args[0] : UndefinedValue());
+    if (!arg.isUndefined()) {
+        if (!ForOf(cx, arg, AddToMap(map)))
+            return false;
+    }
+
+    args.rval().setObject(*obj);
     return true;
 }
 
 #define UNPACK_THIS(T, native, cx, argc, vp, args, data)                      \
     CallArgs args = CallArgsFromVp(argc, vp);                                 \
     if (!args.thisv().isObject() ||                                           \
         !args.thisv().toObject().hasClass(&T::class_))                        \
     {                                                                         \
@@ -360,33 +399,59 @@ SetObject::mark(JSTracer *trc, JSObject 
 void
 SetObject::finalize(JSContext *cx, JSObject *obj)
 {
     SetObject *setobj = static_cast<SetObject *>(obj);
     if (ValueSet *set = setobj->getData())
         cx->delete_(set);
 }
 
+class AddToSet {
+  private:
+    ValueSet *set;
+
+  public:
+    AddToSet(ValueSet *set) : set(set) {}
+
+    bool operator()(JSContext *cx, const Value &v) {
+        HashableValue key;
+        if (!key.setValue(cx, v))
+            return false;
+        if (!set->put(key)) {
+            js_ReportOutOfMemory(cx);
+            return false;
+        }
+        return true;
+    }
+};
+
 JSBool
 SetObject::construct(JSContext *cx, unsigned argc, Value *vp)
 {
     JSObject *obj = NewBuiltinClassInstance(cx, &class_);
     if (!obj)
         return false;
 
     ValueSet *set = cx->new_<ValueSet>(cx->runtime);
     if (!set)
         return false;
     if (!set->init()) {
         js_ReportOutOfMemory(cx);
         return false;
     }
     obj->setPrivate(set);
 
-    CallArgsFromVp(argc, vp).rval().setObject(*obj);
+    CallArgs args = CallArgsFromVp(argc, vp);
+    Value arg = (argc > 0 ? args[0] : UndefinedValue());
+    if (!arg.isUndefined()) {
+        if (!ForOf(cx, arg, AddToSet(set)))
+            return false;
+    }
+
+    args.rval().setObject(*obj);
     return true;
 }
 
 #define THIS_SET(native, cx, argc, vp, args, set)                             \
     UNPACK_THIS(SetObject, native, cx, argc, vp, args, set)
 
 JSBool
 SetObject::size(JSContext *cx, unsigned argc, Value *vp)
rename from js/src/jit-test/tests/collections/Map-size-1.js
rename to js/src/jit-test/tests/collections/Map-constructor-1.js
--- a/js/src/jit-test/tests/collections/Map-size-1.js
+++ b/js/src/jit-test/tests/collections/Map-constructor-1.js
@@ -1,4 +1,6 @@
-// An empty Map has size 0.
+// The Map constructor creates an empty Map by default.
 
 assertEq(Map().size(), 0);
 assertEq((new Map).size(), 0);
+assertEq(Map(undefined).size(), 0);
+assertEq(new Map(undefined).size(), 0);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-2.js
@@ -0,0 +1,6 @@
+// The Map constructor can take an argument that is an array of pairs.
+
+var arr = [["zero", 0], ["one", 1], ["two", 2]];
+var m = Map(arr);
+for (var [k, v] of arr)
+    assertEq(m.get(k), v);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-3.js
@@ -0,0 +1,9 @@
+// Map can take an argument that is an array of singleton arrays.
+
+var arr = [["a"], ["b"], ["c"]];
+var m = Map(arr);
+assertEq(m.size(), 3);
+for (var [k, _] of arr) {
+    assertEq(m.has(k), true);
+    assertEq(m.get(k), undefined);
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-4.js
@@ -0,0 +1,6 @@
+// Map(x) throws if x is not iterable (unless x is undefined).
+
+load(libdir + "asserts.js");
+var nonIterables = [null, true, 1, -0, 3.14, NaN, "", "xyzzy", {}, Math, this];
+for (let k of nonIterables)
+    assertThrowsInstanceOf(function () { Map(k); }, TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-5.js
@@ -0,0 +1,7 @@
+// Map(arr) throws if arr contains holes (or undefined values).
+
+load(libdir + "asserts.js");
+assertThrowsInstanceOf(function () { Map([undefined]); }, TypeError);
+assertThrowsInstanceOf(function () { Map([null]); }, TypeError);
+assertThrowsInstanceOf(function () { Map([[0, 0], [1, 1], , [3, 3]]); }, TypeError);
+assertThrowsInstanceOf(function () { Map([[0, 0], [1, 1], ,]); }, TypeError);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-duplicates.js
@@ -0,0 +1,8 @@
+// When the argument to Map contains a key multiple times, the last value is retained.
+
+var arg = [["zero", 7], ["one", 1], ["two", 4], ["zero", 8], ["two", 2], ["zero", 0]];
+var m = Map(arg);
+assertEq(m.get("zero"), 0);
+assertEq(m.get("one"), 1);
+assertEq(m.get("two"), 2);
+assertEq(m.size(), 3);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-generator-1.js
@@ -0,0 +1,19 @@
+// The argument to Map can be a generator.
+
+var done = false;
+function data(n) {
+    var s = '';
+    for (var i = 0; i < n; i++) {
+        yield [s, i];
+        s += '.';
+    }
+    done = true;
+}
+
+var m = Map(data(50));
+assertEq(done, true);  // the constructor consumes the argument
+assertEq(m.size(), 50);
+assertEq(m.get(""), 0);
+assertEq(m.get("....."), 5);
+assertEq(m.get(Array(49+1).join(".")), 49);
+assertEq(m.has(undefined), false);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-generator-2.js
@@ -0,0 +1,8 @@
+// The argument to Map can be a generator-expression.
+
+var arr = [1, 2, "green", "red"];
+var m = Map([v, v] for (v of arr));
+assertEq(m.size(), 4);
+
+for (var i = 0; i < 4; i++)
+    assertEq(m.get(arr[i]), arr[i]);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-generator-3.js
@@ -0,0 +1,8 @@
+// The argument to Map may be a generator-iterator that produces no values.
+
+assertEq(Map(x for (x of [])).size(), 0);
+
+function none() {
+    if (0) yield 0;
+}
+assertEq(Map(none()).size(), 0);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Map-constructor-generator-exception.js
@@ -0,0 +1,12 @@
+// Iterating over the argument to Map can throw. The exception is propagated.
+
+load(libdir + "asserts.js");
+
+function data2() {
+    yield [{}, "XR22/Z"];
+    yield [{}, "23D-BN"];
+    throw "oops";
+}
+
+var it = data2();
+assertThrowsValue(function () { Map(it); }, "oops");
rename from js/src/jit-test/tests/collections/Map-size-2.js
rename to js/src/jit-test/tests/collections/Map-size.js
--- a/js/src/jit-test/tests/collections/Map-surfaces-1.js
+++ b/js/src/jit-test/tests/collections/Map-surfaces-1.js
@@ -2,17 +2,17 @@
 
 var desc = Object.getOwnPropertyDescriptor(this, "Map");
 assertEq(desc.enumerable, false);
 assertEq(desc.configurable, true);
 assertEq(desc.writable, true);
 
 assertEq(typeof Map, 'function');
 assertEq(Object.keys(Map).length, 0);
-assertEq(Map.length, 0);
+assertEq(Map.length, 1);
 assertEq(Map.name, "Map");
 
 assertEq(Object.getPrototypeOf(Map.prototype), Object.prototype);
 assertEq(Object.prototype.toString.call(Map.prototype), "[object Map]");
 assertEq(Object.prototype.toString.call(new Map), "[object Map]");
 assertEq(Object.prototype.toString.call(Map()), "[object Map]");
 assertEq(Object.keys(Map.prototype).join(), "");
 assertEq(Map.prototype.constructor, Map);
rename from js/src/jit-test/tests/collections/Set-size-1.js
rename to js/src/jit-test/tests/collections/Set-constructor-1.js
--- a/js/src/jit-test/tests/collections/Set-size-1.js
+++ b/js/src/jit-test/tests/collections/Set-constructor-1.js
@@ -1,4 +1,6 @@
-// An empty Set has size 0.
+// The Set constructor creates an empty Set by default.
 
 assertEq(Set().size(), 0);
 assertEq((new Set).size(), 0);
+assertEq(Set(undefined).size(), 0);
+assertEq(new Set(undefined).size(), 0);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Set-constructor-2.js
@@ -0,0 +1,18 @@
+// The Set constructor can take an argument that is an array.
+
+var s = Set([]);
+assertEq(s.size(), 0);
+assertEq(s.has(undefined), false);
+
+s = Set(["one", "two", "three"]);
+assertEq(s.size(), 3);
+assertEq(s.has("one"), true);
+assertEq(s.has("eleventeen"), false);
+
+var a = [{}, {}, {}];
+s = Set(a);
+assertEq(s.size(), 3);
+for (let obj of a)
+    assertEq(s.has(obj), true);
+assertEq(s.has({}), false);
+assertEq(s.has("three"), false);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Set-constructor-3.js
@@ -0,0 +1,11 @@
+// The argument to Set may contain a value multiple times. Duplicates are discarded.
+
+assertEq(Set(["testing", "testing", 123]).size(), 2);
+
+var values = [undefined, null, false, NaN, 0, -0, 6.022e23, -Infinity, "", "xyzzy", {}, Math.sin];
+for (let v of values) {
+    var a = [v, {}, {}, {}, v, {}, v, v];
+    var s = Set(a);
+    assertEq(s.size(), 5);
+    assertEq(s.has(v), true);
+}
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Set-constructor-generator-1.js
@@ -0,0 +1,12 @@
+// The argument to Set can be a generator.
+
+function hexData(n) {
+    for (var i = 0; i < n; i++)
+        yield i.toString(16);
+}
+
+var s = Set(hexData(256));
+assertEq(s.size(), 256);
+assertEq(s.has("0"), true);
+assertEq(s.has(0), false);
+assertEq(s.has("ff"), true);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/collections/Set-constructor-generator-2.js
@@ -0,0 +1,8 @@
+// The argument to Set can be a generator-expression.
+
+var s = Set(k * k for (k of [1, 2, 3, 4]));
+assertEq(s.size(), 4);
+assertEq(s.has(1), true);
+assertEq(s.has(4), true);
+assertEq(s.has(9), true);
+assertEq(s.has(16), true);
rename from js/src/jit-test/tests/collections/Set-size-2.js
rename to js/src/jit-test/tests/collections/Set-size.js
--- a/js/src/jit-test/tests/collections/Set-surfaces-1.js
+++ b/js/src/jit-test/tests/collections/Set-surfaces-1.js
@@ -2,17 +2,17 @@
 
 var desc = Object.getOwnPropertyDescriptor(this, "Set");
 assertEq(desc.enumerable, false);
 assertEq(desc.configurable, true);
 assertEq(desc.writable, true);
 
 assertEq(typeof Set, 'function');
 assertEq(Object.keys(Set).length, 0);
-assertEq(Set.length, 0);
+assertEq(Set.length, 1);
 assertEq(Set.name, "Set");
 
 assertEq(Object.getPrototypeOf(Set.prototype), Object.prototype);
 assertEq(Object.prototype.toString.call(Set.prototype), "[object Set]");
 assertEq(Object.prototype.toString.call(new Set), "[object Set]");
 assertEq(Object.prototype.toString.call(Set()), "[object Set]");
 assertEq(Object.keys(Set.prototype).join(), "");
 assertEq(Set.prototype.constructor, Set);
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -472,17 +472,17 @@ GetCustomIterator(JSContext *cx, JSObjec
         ++sCustomIteratorCount;
 
     /* Otherwise call it and return that object. */
     Value arg = BooleanValue((flags & JSITER_FOREACH) == 0);
     if (!Invoke(cx, ObjectValue(*obj), *vp, 1, &arg, vp))
         return false;
     if (vp->isPrimitive()) {
         /*
-         * We are always coming from js_ValueToIterator, and we are no longer on
+         * We are always coming from js::ValueToIterator, and we are no longer on
          * trace, so the object we are iterating over is on top of the stack (-1).
          */
         JSAutoByteString bytes;
         if (!js_AtomToPrintableString(cx, atom, &bytes))
             return false;
         js_ReportValueError2(cx, JSMSG_BAD_TRAP_RETURN_VALUE,
                              -1, ObjectValue(*obj), NULL, bytes.ptr());
         return false;
--- a/js/src/jsiter.h
+++ b/js/src/jsiter.h
@@ -178,17 +178,17 @@ VectorToValueIterator(JSContext *cx, JSO
  * Creates either a key or value iterator, depending on flags. For a value
  * iterator, performs value-lookup to convert the given list of jsids.
  */
 bool
 EnumeratedIdVectorToIterator(JSContext *cx, JSObject *obj, unsigned flags, js::AutoIdVector &props, js::Value *vp);
 
 /*
  * Convert the value stored in *vp to its iteration object. The flags should
- * contain JSITER_ENUMERATE if js_ValueToIterator is called when enumerating
+ * contain JSITER_ENUMERATE if js::ValueToIterator is called when enumerating
  * for-in semantics are required, and when the caller can guarantee that the
  * iterator will never be exposed to scripts.
  */
 extern JSBool
 ValueToIterator(JSContext *cx, unsigned flags, js::Value *vp);
 
 extern bool
 CloseIterator(JSContext *cx, JSObject *iterObj);
@@ -216,16 +216,81 @@ extern JSBool
 js_IteratorMore(JSContext *cx, JSObject *iterobj, js::Value *rval);
 
 extern JSBool
 js_IteratorNext(JSContext *cx, JSObject *iterobj, js::Value *rval);
 
 extern JSBool
 js_ThrowStopIteration(JSContext *cx);
 
+namespace js {
+
+/*
+ * Get the next value from an iterator object.
+ *
+ * On success, store the next value in *vp and return true; if there are no
+ * more values, store the magic value JS_NO_ITER_VALUE in *vp and return true.
+ */
+inline bool
+Next(JSContext *cx, JSObject *iter, Value *vp)
+{
+    if (!js_IteratorMore(cx, iter, vp))
+        return false;
+    if (vp->toBoolean())
+        return js_IteratorNext(cx, iter, vp);
+    vp->setMagic(JS_NO_ITER_VALUE);
+    return true;
+}
+
+/*
+ * Imitate a for-of loop. This does the equivalent of the JS code:
+ *
+ *     for (let v of iterable)
+ *         op(v);
+ *
+ * But the actual signature of op must be:
+ *     bool op(JSContext *cx, const Value &v);
+ *
+ * There is no feature like JS 'break'. op must return false only
+ * in case of exception or error.
+ */
+template <class Op>
+bool
+ForOf(JSContext *cx, const Value &iterable, Op op)
+{
+    Value iterv(iterable);
+    if (!ValueToIterator(cx, JSITER_FOR_OF, &iterv))
+        return false;
+    JSObject *iter = &iterv.toObject();
+
+    bool ok = true;
+    while (ok) {
+        Value v;
+        ok = Next(cx, iter, &v);
+        if (ok) {
+            if (v.isMagic(JS_NO_ITER_VALUE))
+                break;
+            ok = op(cx, v);
+        }
+    }
+
+    bool throwing = !ok && cx->isExceptionPending();
+    Value exc;
+    if (throwing) {
+        exc = cx->getPendingException();
+        cx->clearPendingException();
+    }
+    bool closedOK = CloseIterator(cx, iter);
+    if (throwing && closedOK)
+        cx->setPendingException(exc);
+    return ok && closedOK;
+}
+
+} /* namespace js */
+
 #if JS_HAS_GENERATORS
 
 /*
  * Generator state codes.
  */
 typedef enum JSGeneratorState {
     JSGEN_NEWBORN,  /* not yet started */
     JSGEN_OPEN,     /* started by a .next() or .send(undefined) call */