Bug 904723, part 4 - In Array.from, only fetch the @@iterator property once. r=till. DONTBUILD.
authorJason Orendorff <jorendorff@mozilla.com>
Fri, 06 Jun 2014 14:13:16 -0400
changeset 207558 0354269b906b0e7fa389df630e2f70aa8310900a
parent 207557 3a892952450c8c7a8948b98334c79fe14716bb05
child 207559 078b39cfcf44bb6513d3ffbd2f38ecd00c4bce94
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstill
bugs904723
milestone32.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 904723, part 4 - In Array.from, only fetch the @@iterator property once. r=till. DONTBUILD.
CLOBBER
js/src/builtin/Array.js
js/src/js.msg
js/src/tests/ecma_6/Array/from_errors.js
js/src/tests/ecma_6/Array/from_proxy.js
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,10 +17,10 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Bug 904723 (Array.from) needs clobber because of changes to js.msg and
-self-hosted code (see bug 1019955).
+Bug 904723 (Array.from) needs a clobber once again because of changes to js.msg
+and self-hosted code (see bug 1019955).
--- a/js/src/builtin/Array.js
+++ b/js/src/builtin/Array.js
@@ -655,50 +655,63 @@ function ArrayValues() {
 function ArrayEntries() {
     return CreateArrayIterator(this, ITEM_KIND_KEY_AND_VALUE);
 }
 
 function ArrayKeys() {
     return CreateArrayIterator(this, ITEM_KIND_KEY);
 }
 
-/* ES6 rev 24 (2014 April 27) 22.1.2.1 */
+/* ES6 rev 25 (2014 May 22) 22.1.2.1 */
 function ArrayFrom(arrayLike, mapfn=undefined, thisArg=undefined) {
     // Step 1.
     var C = this;
 
     // Steps 2-3.
     var items = ToObject(arrayLike);
 
     // Steps 4-5.
     var mapping = (mapfn !== undefined);
     if (mapping && !IsCallable(mapfn))
         ThrowError(JSMSG_NOT_FUNCTION, DecompileArg(1, mapfn));
 
     // All elements defined by this algorithm have the same attrs:
     var attrs = ATTR_CONFIGURABLE | ATTR_ENUMERABLE | ATTR_WRITABLE;
 
     // Steps 6-8.
-    if (items["@@iterator"] !== undefined) {
+    var usingIterator = items["@@iterator"];
+    if (usingIterator !== undefined) {
         // Steps 8.a-c.
         var A = IsConstructor(C) ? new C() : [];
 
+        // Steps 8.d-e.
+        var iterator = callFunction(usingIterator, items);
+
         // Step 8.f.
         var k = 0;
 
-        // Steps 8.d-e and 8.g.i-vi.
-        for (var nextValue of items) {
+        // Steps 8.g.i-vi.
+        // These steps cannot be implemented using a for-of loop.
+        // See <https://bugs.ecmascript.org/show_bug.cgi?id=2883>.
+        var next;
+        while (true) {
+            // Steps 8.g.ii-vi.
+            next = iterator.next();
+            if (!IsObject(next))
+                ThrowError(JSMSG_NEXT_RETURNED_PRIMITIVE);
+            if (next.done)
+                break;  // Substeps of 8.g.iv are implemented below.
+            var nextValue = next.value;
+
             // Steps 8.g.vii-viii.
             var mappedValue = mapping ? callFunction(mapfn, thisArg, nextValue, k) : nextValue;
 
             // Steps 8.g.ix-xi.
             _DefineDataProperty(A, k++, mappedValue, attrs);
         }
-
-        // Here we're at step 8.g.iv.1. Fall through; it's implemented below.
     } else {
         // Step 9 is an assertion: items is not an Iterator. Testing this is
         // literally the very last thing we did, so we don't assert here.
 
         // Steps 10-12.
         // FIXME: Array operations should use ToLength (bug 924058).
         var len = ToInteger(items.length);
 
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -352,17 +352,17 @@ MSG_DEF(JSMSG_FUNCTION_ARGUMENTS_AND_RES
 MSG_DEF(JSMSG_REST_WITH_DEFAULT,      299, 0, JSEXN_SYNTAXERR, "rest parameter may not have a default")
 MSG_DEF(JSMSG_NONDEFAULT_FORMAL_AFTER_DEFAULT, 300, 0, JSEXN_SYNTAXERR, "parameter(s) with default followed by parameter without default")
 MSG_DEF(JSMSG_YIELD_IN_DEFAULT,       301, 0, JSEXN_SYNTAXERR, "yield in default expression")
 MSG_DEF(JSMSG_INTRINSIC_NOT_DEFINED,  302, 1, JSEXN_REFERENCEERR, "no intrinsic function {0}")
 MSG_DEF(JSMSG_ALREADY_HAS_PRAGMA, 303, 2, JSEXN_ERR,      "{0} is being assigned a {1}, but already has one")
 MSG_DEF(JSMSG_PAR_ARRAY_BAD_ARG,      304, 0, JSEXN_RANGEERR, "invalid parallel method argument")
 MSG_DEF(JSMSG_REGEXP_RUNTIME_ERROR,   305, 0, JSEXN_INTERNALERR, "an error occurred while executing regular expression")
 MSG_DEF(JSMSG_DEBUG_OPTIMIZED_OUT,    306, 0, JSEXN_ERR, "variable has been optimized out")
-MSG_DEF(JSMSG_UNUSED307,              307, 0, JSEXN_NONE, "")
+MSG_DEF(JSMSG_NEXT_RETURNED_PRIMITIVE,307, 0, JSEXN_TYPEERR, "iterator.next() returned a non-object value")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_CONFLICT, 308, 0, JSEXN_ERR, "no conflict resolution function provided")
 MSG_DEF(JSMSG_PAR_ARRAY_SCATTER_BOUNDS, 309, 0, JSEXN_ERR, "index in scatter vector out of bounds")
 MSG_DEF(JSMSG_CANT_REPORT_NC_AS_NE,   310, 0, JSEXN_TYPEERR, "proxy can't report a non-configurable own property as non-existent")
 MSG_DEF(JSMSG_CANT_REPORT_E_AS_NE,    311, 0, JSEXN_TYPEERR, "proxy can't report an existing own property as non-existent on a non-extensible object")
 MSG_DEF(JSMSG_CANT_REPORT_NEW,        312, 0, JSEXN_TYPEERR, "proxy can't report a new property on a non-extensible object")
 MSG_DEF(JSMSG_CANT_REPORT_INVALID,    313, 0, JSEXN_TYPEERR, "proxy can't report an incompatible property descriptor")
 MSG_DEF(JSMSG_CANT_REPORT_NE_AS_NC,   314, 0, JSEXN_TYPEERR, "proxy can't report a non-existent property as non-configurable")
 MSG_DEF(JSMSG_CANT_DEFINE_NEW,        315, 0, JSEXN_TYPEERR, "proxy can't define a new property on a non-extensible object")
--- a/js/src/tests/ecma_6/Array/from_errors.js
+++ b/js/src/tests/ecma_6/Array/from_errors.js
@@ -125,10 +125,21 @@ var arrayish = {
     get 0() { log += "0"; return "q"; }
 };
 log = "";
 var exc = {surprise: "ponies"};
 assertThrowsValue(() => Array.from.call(C, arrayish, () => { throw exc; }), exc);
 assertEq(log, "lC0");
 assertEq(obj instanceof C, true);
 
+// It's a TypeError if the iterator's .next() method returns a primitive.
+for (var primitive of [undefined, null, 17]) {
+    assertThrowsInstanceOf(
+        () => Array.from({
+            "@@iterator": () => {
+                next: () => primitive
+            }
+        }),
+        TypeError);
+}
+
 if (typeof reportCompare === 'function')
     reportCompare(0, 0);
--- a/js/src/tests/ecma_6/Array/from_proxy.js
+++ b/js/src/tests/ecma_6/Array/from_proxy.js
@@ -31,14 +31,22 @@ function LoggingProxy(target) {
 LoggingProxy.from = Array.from;
 LoggingProxy.from([3, 4, 5]);
 assertDeepEq(log, ["define 0", "define 1", "define 2", "set length"]);
 
 // When the argument passed to Array.from is a Proxy, Array.from
 // calls handler.get on it.
 log = [];
 assertDeepEq(Array.from(new LoggingProxy([3, 4, 5])), [3, 4, 5]);
-assertDeepEq(log, ["get @@iterator", "get @@iterator",
+assertDeepEq(log, ["get @@iterator",
                    "get length", "get 0", "get length", "get 1", "get length", "get 2",
                    "get length"]);
 
+// Array-like iteration only gets the length once.
+log = [];
+var arr = [5, 6, 7];
+arr["@@iterator"] = undefined;
+assertDeepEq(Array.from(new LoggingProxy(arr)), [5, 6, 7]);
+assertDeepEq(log, ["get @@iterator",
+                   "get length", "get 0", "get 1", "get 2"]);
+
 if (typeof reportCompare === 'function')
     reportCompare(0, 0);