Bug 1067049 - Implement arguments[@@iterator]. r=evilpie
authorTooru Fujisawa <arai_a@mac.com>
Wed, 06 Jan 2016 17:53:21 +0900
changeset 278808 f7aa719c43c9639680f4d525b4ad7175e93b459d
parent 278712 4103127ecd874a32a5583e13c98668cfe6d50ae1
child 278809 b51771070a8993305e2e73a31bc3e82c32de3ae4
push id29860
push usercbook@mozilla.com
push dateThu, 07 Jan 2016 10:51:20 +0000
treeherdermozilla-central@e0bcd16e1d4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1067049
milestone46.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 1067049 - Implement arguments[@@iterator]. r=evilpie
js/src/jit-test/tests/collections/constructor-errors.js
js/src/jit-test/tests/for-of/non-iterable.js
js/src/jit-test/tests/ion/bug832058.js
js/src/tests/ecma_6/Function/arguments-iterator.js
js/src/vm/ArgumentsObject.cpp
js/src/vm/ArgumentsObject.h
js/src/vm/NativeObject.cpp
--- a/js/src/jit-test/tests/collections/constructor-errors.js
+++ b/js/src/jit-test/tests/collections/constructor-errors.js
@@ -1,22 +1,19 @@
 // The Set constructor throws TypeError when passed a non-iterable argument.
 
 load(libdir + "asserts.js");
 
-function argsobj() { return arguments; }
-
 var misc = [
     {}, {x: 1}, Math, isNaN,
     Object.create(null),
-    argsobj(0, 1, 2),
     true, 0, 3.1416,
     new Boolean(true), new Number(0),
     {iterator: function () { return undefined; }},
     {iterator: function () { return null; }},
     {iterator: function () { return true; }},
     {iterator: function () { return 17; }},
 ];
 
 for (var v of misc) {
     assertThrowsInstanceOf(function () { new Set(v); }, TypeError);
     assertThrowsInstanceOf(function () { new Map(v); }, TypeError);
-}
\ No newline at end of file
+}
--- a/js/src/jit-test/tests/for-of/non-iterable.js
+++ b/js/src/jit-test/tests/for-of/non-iterable.js
@@ -1,18 +1,15 @@
 // Iterating over non-iterable values throws a TypeError.
 
 load(libdir + "asserts.js");
 
-function argsobj() { return arguments; }
-
 var misc = [
     {}, {x: 1}, Math, isNaN,
     Object.create(null),
-    argsobj(0, 1, 2),
     null, undefined,
     true, 0, 3.1416,
     new Boolean(true), new Number(0),
     {iterator: function () { return undefined; }},
     {iterator: function () { return null; }},
     {iterator: function () { return true; }},
     {iterator: function () { return 17; }},
 ];
--- a/js/src/jit-test/tests/ion/bug832058.js
+++ b/js/src/jit-test/tests/ion/bug832058.js
@@ -1,15 +1,16 @@
-// |jit-test| error: TypeError
+// |jit-test|
 function f(c) {
   var b = arguments;
   if (c == 1)
     b = 1;
   return b;
 }
 
 evaluate("f('a', 'b', 'c', 'd', 'e');");
 function test(){
   var args = f('a', (0), 'c');
+  var s;
   for (var v of args)
       s += v;
 }
 test();
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/Function/arguments-iterator.js
@@ -0,0 +1,167 @@
+var BUGNUMBER = 992617;
+var summary = "Implement arguments[@@iterator].";
+
+print(BUGNUMBER + ": " + summary);
+
+// MappedArgumentsObject
+let mapped = [
+  function(a, b, c) {
+    assertEq(Symbol.iterator in arguments, true);
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function(a, b, c) {
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function(a, b, c) {
+    arguments[Symbol.iterator] = 10;
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function(a, b, c) {
+    Object.defineProperty(arguments, Symbol.iterator, {
+      value: 10, writable: true, enumerable: true, configurable: true
+    });
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function(a, b, c) {
+    assertEq(arguments[Symbol.iterator], Array.prototype[Symbol.iterator]);
+  },
+  function(a, b, c) {
+    assertEq(arguments[Symbol.iterator].name, "values");
+  },
+  function(a, b, c) {
+    var desc = Object.getOwnPropertyDescriptor(arguments, Symbol.iterator);
+    assertEq("value" in desc, true);
+    assertEq(desc.value, Array.prototype[Symbol.iterator]);
+    assertEq(desc.writable, true);
+    assertEq(desc.enumerable, false);
+    assertEq(desc.configurable, true);
+  },
+  function(a, b, c) {
+    var iter = arguments[Symbol.iterator]();
+    assertDeepEq(iter.next(), { value: 10, done: false });
+    assertDeepEq(iter.next(), { value: 20, done: false });
+    assertDeepEq(iter.next(), { value: 30, done: false });
+    assertDeepEq(iter.next(), { value: undefined, done: true });
+  },
+  function(a, b, c) {
+    assertDeepEq([...arguments], [10, 20, 30]);
+  },
+  function(a, b, c) {
+    b = 40;
+    assertDeepEq([...arguments], [10, 40, 30]);
+  },
+  function(a, b, c) {
+    arguments.length = 4;
+    assertDeepEq([...arguments], [10, 20, 30, undefined]);
+  },
+  function(a, b, c) {
+    arguments[5] = 50;
+    assertDeepEq([...arguments], [10, 20, 30]);
+  },
+  function(a, b, c) {
+    arguments[Symbol.iterator] = function*() {
+      yield 40;
+      yield 50;
+      yield 60;
+    };
+    assertDeepEq([...arguments], [40, 50, 60]);
+  },
+];
+for (let f of mapped) {
+  f(10, 20, 30);
+}
+
+var g1 = newGlobal();
+assertEq(g1.eval(`
+function f(a, b, c) {
+  return arguments[Symbol.iterator].name;
+}
+f(1, 2, 3);
+`), "values");
+
+// UnmappedArgumentsObject
+let unmapped = [
+  function([a], b, c) {
+    assertEq(Symbol.iterator in arguments, true);
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function([a], b, c) {
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function([a], b, c) {
+    arguments[Symbol.iterator] = 10;
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function([a], b, c) {
+    Object.defineProperty(arguments, Symbol.iterator, {
+      value: 10, writable: true, enumerable: true, configurable: true
+    });
+    delete arguments[Symbol.iterator];
+    assertEq(Symbol.iterator in arguments, false);
+  },
+  function([a], b, c) {
+    assertEq(arguments[Symbol.iterator], Array.prototype[Symbol.iterator]);
+  },
+  function([a], b, c) {
+    assertEq(arguments[Symbol.iterator].name, "values");
+  },
+  function([a], b, c) {
+    var desc = Object.getOwnPropertyDescriptor(arguments, Symbol.iterator);
+    assertEq("value" in desc, true);
+    assertEq(desc.value, Array.prototype[Symbol.iterator]);
+    assertEq(desc.writable, true);
+    assertEq(desc.enumerable, false);
+    assertEq(desc.configurable, true);
+  },
+  function([a], b, c) {
+    var iter = arguments[Symbol.iterator]();
+    assertDeepEq(iter.next(), { value: [10], done: false });
+    assertDeepEq(iter.next(), { value: 20, done: false });
+    assertDeepEq(iter.next(), { value: 30, done: false });
+    assertDeepEq(iter.next(), { value: undefined, done: true });
+  },
+  function([a], b, c) {
+    assertDeepEq([...arguments], [[10], 20, 30]);
+  },
+  function([a], b, c) {
+    b = 40;
+    assertDeepEq([...arguments], [[10], 20, 30]);
+  },
+  function([a], b, c) {
+    arguments.length = 4;
+    assertDeepEq([...arguments], [[10], 20, 30, undefined]);
+  },
+  function([a], b, c) {
+    arguments[5] = 50;
+    assertDeepEq([...arguments], [[10], 20, 30]);
+  },
+  function([a], b, c) {
+    arguments[Symbol.iterator] = function*() {
+      yield 40;
+      yield 50;
+      yield 60;
+    };
+    assertDeepEq([...arguments], [40, 50, 60]);
+  },
+];
+for (let f of unmapped) {
+  f([10], 20, 30);
+}
+
+var g2 = newGlobal();
+assertEq(g2.eval(`
+function f([a], b, c) {
+  return arguments[Symbol.iterator].name;
+}
+f([1], 2, 3);
+`), "values");
+
+if (typeof reportCompare === "function")
+    reportCompare(true, true);
--- a/js/src/vm/ArgumentsObject.cpp
+++ b/js/src/vm/ArgumentsObject.cpp
@@ -323,16 +323,18 @@ ArgumentsObject::obj_delProperty(JSConte
     if (JSID_IS_INT(id)) {
         unsigned arg = unsigned(JSID_TO_INT(id));
         if (arg < argsobj.initialLength() && !argsobj.isElementDeleted(arg))
             argsobj.markElementDeleted(arg);
     } else if (JSID_IS_ATOM(id, cx->names().length)) {
         argsobj.markLengthOverridden();
     } else if (JSID_IS_ATOM(id, cx->names().callee)) {
         argsobj.as<MappedArgumentsObject>().clearCallee();
+    } else if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
+        argsobj.markIteratorOverridden();
     }
     return result.succeed();
 }
 
 static bool
 MappedArgGetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue vp)
 {
     MappedArgumentsObject& argsobj = obj->as<MappedArgumentsObject>();
@@ -393,21 +395,44 @@ MappedArgSetter(JSContext* cx, HandleObj
      * the user has changed the prototype to an object that has a setter for
      * this id.
      */
     ObjectOpResult ignored;
     return NativeDeleteProperty(cx, argsobj, id, ignored) &&
            NativeDefineProperty(cx, argsobj, id, vp, nullptr, nullptr, attrs, result);
 }
 
+static bool
+DefineArgumentsIterator(JSContext* cx, Handle<ArgumentsObject*> argsobj)
+{
+    RootedId iteratorId(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator));
+    HandlePropertyName shName = cx->names().ArrayValues;
+    RootedAtom name(cx, cx->names().values);
+    RootedValue val(cx);
+    if (!GlobalObject::getSelfHostedFunction(cx, cx->global(), shName, name, 0, &val))
+        return false;
+
+    return NativeDefineProperty(cx, argsobj, iteratorId, val, nullptr, nullptr, JSPROP_RESOLVING);
+}
+
 /* static */ bool
 MappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
 {
     Rooted<MappedArgumentsObject*> argsobj(cx, &obj->as<MappedArgumentsObject>());
 
+    if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
+        if (argsobj->hasOverriddenIterator())
+            return true;
+
+        if (!DefineArgumentsIterator(cx, argsobj))
+            return false;
+        *resolvedp = true;
+        return true;
+    }
+
     unsigned attrs = JSPROP_SHARED | JSPROP_SHADOWABLE | JSPROP_RESOLVING;
     if (JSID_IS_INT(id)) {
         uint32_t arg = uint32_t(JSID_TO_INT(id));
         if (arg >= argsobj->initialLength() || argsobj->isElementDeleted(arg))
             return true;
 
         attrs |= JSPROP_ENUMERATE;
     } else if (JSID_IS_ATOM(id, cx->names().length)) {
@@ -443,16 +468,20 @@ MappedArgumentsObject::obj_enumerate(JSC
     id = NameToId(cx->names().length);
     if (!HasProperty(cx, argsobj, id, &found))
         return false;
 
     id = NameToId(cx->names().callee);
     if (!HasProperty(cx, argsobj, id, &found))
         return false;
 
+    id = SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator);
+    if (!HasProperty(cx, argsobj, id, &found))
+        return false;
+
     for (unsigned i = 0; i < argsobj->initialLength(); i++) {
         id = INT_TO_JSID(i);
         if (!HasProperty(cx, argsobj, id, &found))
             return false;
     }
 
     return true;
 }
@@ -514,16 +543,26 @@ UnmappedArgSetter(JSContext* cx, HandleO
            NativeDefineProperty(cx, argsobj, id, vp, nullptr, nullptr, attrs, result);
 }
 
 /* static */ bool
 UnmappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
 {
     Rooted<UnmappedArgumentsObject*> argsobj(cx, &obj->as<UnmappedArgumentsObject>());
 
+    if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
+        if (argsobj->hasOverriddenIterator())
+            return true;
+
+        if (!DefineArgumentsIterator(cx, argsobj))
+            return false;
+        *resolvedp = true;
+        return true;
+    }
+
     unsigned attrs = JSPROP_SHARED | JSPROP_SHADOWABLE;
     GetterOp getter = UnmappedArgGetter;
     SetterOp setter = UnmappedArgSetter;
 
     if (JSID_IS_INT(id)) {
         uint32_t arg = uint32_t(JSID_TO_INT(id));
         if (arg >= argsobj->initialLength() || argsobj->isElementDeleted(arg))
             return true;
@@ -565,16 +604,20 @@ UnmappedArgumentsObject::obj_enumerate(J
     id = NameToId(cx->names().callee);
     if (!HasProperty(cx, argsobj, id, &found))
         return false;
 
     id = NameToId(cx->names().caller);
     if (!HasProperty(cx, argsobj, id, &found))
         return false;
 
+    id = SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator);
+    if (!HasProperty(cx, argsobj, id, &found))
+        return false;
+
     for (unsigned i = 0; i < argsobj->initialLength(); i++) {
         id = INT_TO_JSID(i);
         if (!HasProperty(cx, argsobj, id, &found))
             return false;
     }
 
     return true;
 }
--- a/js/src/vm/ArgumentsObject.h
+++ b/js/src/vm/ArgumentsObject.h
@@ -104,33 +104,35 @@ static const unsigned ARGS_LENGTH_MAX = 
  *
  * ES5's strict mode behaves more sanely, and named arguments don't alias
  * elements of an arguments object.
  *
  * ArgumentsObject instances use the following reserved slots:
  *
  *   INITIAL_LENGTH_SLOT
  *     Stores the initial value of arguments.length, plus a bit indicating
- *     whether arguments.length has been modified.  Use initialLength() and
- *     hasOverriddenLength() to access these values.  If arguments.length has
+ *     whether arguments.length and/or arguments[@@iterator] have been
+ *     modified.  Use initialLength(), hasOverriddenLength(), and
+ *     hasOverriddenIterator() to access these values.  If arguments.length has
  *     been modified, then the current value of arguments.length is stored in
  *     another slot associated with a new property.
  *   DATA_SLOT
  *     Stores an ArgumentsData*, described above.
  */
 class ArgumentsObject : public NativeObject
 {
   protected:
     static const uint32_t INITIAL_LENGTH_SLOT = 0;
     static const uint32_t DATA_SLOT = 1;
     static const uint32_t MAYBE_CALL_SLOT = 2;
 
   public:
     static const uint32_t LENGTH_OVERRIDDEN_BIT = 0x1;
-    static const uint32_t PACKED_BITS_COUNT = 1;
+    static const uint32_t ITERATOR_OVERRIDDEN_BIT = 0x2;
+    static const uint32_t PACKED_BITS_COUNT = 2;
 
   protected:
     template <typename CopyArgs>
     static ArgumentsObject* create(JSContext* cx, HandleFunction callee, unsigned numActuals,
                                    CopyArgs& copy);
 
     ArgumentsData* data() const {
         return reinterpret_cast<ArgumentsData*>(getFixedSlot(DATA_SLOT).toPrivate());
@@ -180,16 +182,28 @@ class ArgumentsObject : public NativeObj
         return v.toInt32() & LENGTH_OVERRIDDEN_BIT;
     }
 
     void markLengthOverridden() {
         uint32_t v = getFixedSlot(INITIAL_LENGTH_SLOT).toInt32() | LENGTH_OVERRIDDEN_BIT;
         setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v));
     }
 
+    /* True iff arguments[@@iterator] has been assigned or its attributes
+     * changed. */
+    bool hasOverriddenIterator() const {
+        const Value& v = getFixedSlot(INITIAL_LENGTH_SLOT);
+        return v.toInt32() & ITERATOR_OVERRIDDEN_BIT;
+    }
+
+    void markIteratorOverridden() {
+        uint32_t v = getFixedSlot(INITIAL_LENGTH_SLOT).toInt32() | ITERATOR_OVERRIDDEN_BIT;
+        setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v));
+    }
+
     /*
      * Because the arguments object is a real object, its elements may be
      * deleted. This is implemented by setting a 'deleted' flag for the arg
      * which is read by argument object resolve and getter/setter hooks.
      *
      * NB: an element, once deleted, stays deleted. Thus:
      *
      *   function f(x) { delete arguments[0]; arguments[0] = 42; return x }
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1340,16 +1340,21 @@ js::NativeDefineProperty(ExclusiveContex
             // Either we are resolving the .length property on this object, or
             // redefining it. In the latter case only, we must set a bit. To
             // distinguish the two cases, we note that when resolving, the
             // property won't already exist; whereas the first time it is
             // redefined, it will.
             if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
                 obj->as<ArgumentsObject>().markLengthOverridden();
         }
+        if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
+            // Do same thing as .length for [@@iterator].
+            if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
+                obj->as<ArgumentsObject>().markIteratorOverridden();
+        }
     }
 
     // 9.1.6.1 OrdinaryDefineOwnProperty steps 1-2.
     RootedShape shape(cx);
     if (desc_.attributes() & JSPROP_RESOLVING) {
         // We are being called from a resolve or enumerate hook to reify a
         // lazily-resolved property. To avoid reentering the resolve hook and
         // recursing forever, skip the resolve hook when doing this lookup.