Implement Array.from()

src/harmony-array.js

 // Copyright 2013 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 'use strict';
 
 // This file relies on the fact that the following declaration has been made
 // in runtime.js:
 // var $Array = global.Array;
 
@@ skipping 105 matching lines @@
   if ((end - i) > 0 && ObjectIsFrozen(array)) {
     throw MakeTypeError("array_functions_on_frozen",
                         ["Array.prototype.fill"]);
   }
 
   for (; i < end; i++)
     array[i] = value;
   return array;
 }
 
+// ES6, draft 08-24-14, section 22.1.2.1
+function ArrayFrom(arrayLike, mapfn, receiver) {
+  var C = this;

[rossberg, 2014/09/10 07:23:55]

Nit: drop this redundant variable.

+  var items = ToObject(arrayLike);
+  var mapping;
+
+  if (IS_UNDEFINED(mapfn)) {

[rossberg, 2014/09/10 07:23:55]

var mapping = !IS_UNDEFINED(mapfn)

But in fact, it's redundant to define this variable at all.

+    mapping = false;
+  } else {
+    mapping = true;
+    if (!IS_SPEC_FUNCTION(mapfn)) {
+      throw MakeTypeError('called_non_callable', [ mapfn ]);
+    }
+
+    if (IS_NULL_OR_UNDEFINED(receiver)) {
+      receiver = %GetDefaultReceiver(mapfn) || receiver;
+    } else if (!IS_SPEC_OBJECT(receiver) && %IsSloppyModeFunction(mapfn)) {
+      receiver = ToObject(receiver);

[rossberg, 2014/09/10 07:23:55]

Hm, is this actually necessary? Doesn't _CallFunction do it already?

[caitp (gmail), 2014/09/10 14:26:04]

Looks like Execution::Call() does this, yes --- edit, no, apparently this does
not do the right thing, so we do actually need to set this up in the script.
(For sloppy-mode, Execution::Call is not passing the correct global variable
when null/undefined is used, as it looks like it should --- this causes test
failures).

[arv (Not doing code reviews), 2014/09/10 14:33:58]

Yes. We don't want _CallFunction to have to do the extra work to do this in
general. That is why it is up to all the callers to handle it.

We could provide a helper function that does this, which would clean up the
repeated code we already have.

+    }
+  }
+
+  var usingIterator = ToIterable(items);

[rossberg, 2014/09/10 07:23:55]

Nit: rename to iterable?

+  var k;

[rossberg, 2014/09/10 07:23:55]

These can all be locally declared. Also, please use more descriptive names, even
if the spec doesn't (A => result).

[caitp (gmail), 2014/09/10 14:26:04]

More descriptive name is good --- I think declaring these outside of the blocks
which use them is good in this case --- it's redundant to define them locally
for both blocks, and confusing  if they're defined in one local block and the
second block depends on the hoisted variable.

+  var A;
+  var mappedValue;
+  var nextValue;
+
+  if (!IS_UNDEFINED(usingIterator)) {
+    // Step 8.
+    A = IS_SPEC_FUNCTION(C) ? new C() : [];
+
+    var iterator = GetIterator(items, usingIterator);
+    var next;
+    k = 0;
+
+    while (!(next = iterator.next()).done) {

[rossberg, 2014/09/10 07:23:55]

I think

  for (var next = iterator.next(); !next.done; next = iterator.next())

would be much clearer. In fact, we shipped iterators, so you should be able to
say just

  for (var next of iterable)

and remove much of the boilerplate here... unless I'm missing something.

[arv (Not doing code reviews), 2014/09/10 14:05:14]

Using for-of is going to be an observable difference due to IteratorClose which
checks and calls .return in case of an abrubt completion

+      if (!IS_SPEC_OBJECT(next)) {
+        throw MakeTypeError('iterator_result_not_an_object', [next]);
+      }
+      nextValue = next.value;
+      mappedValue = mapping ? %_CallFunction(receiver, nextValue, k, mapfn) : nextValue;

[rossberg, 2014/09/10 07:23:55]

Nit: line length

+      %AddElement(A, k++, mappedValue, NONE);
+    }
+    A.length = k;
+    return A;
+  } else {
+    // Steps 9 .. 20
+    var len = TO_LENGTH(items.length);
+    A = IS_SPEC_FUNCTION(C) ? new C(len) : new $Array(len);
+
+    for (k = 0; k < len; ++k) {
+      nextValue = items[k];
+      mappedValue = mapping ? %_CallFunction(receiver, nextValue, k, mapfn) : nextValue;

[rossberg, 2014/09/10 07:23:55]

Nit: line length

+      %AddElement(A, k, mappedValue, NONE);
+    }
+
+    A.length = k;
+    return A;
+  }
+}
+
 // ES6, draft 05-22-14, section 22.1.2.3
 function ArrayOf() {
   var length = %_ArgumentsLength();
   var constructor = this;
   // TODO: Implement IsConstructor (ES6 section 7.2.5)
   var array = IS_SPEC_FUNCTION(constructor) ? new constructor(length) : [];
   for (var i = 0; i < length; i++) {
     %AddElement(array, i, %_Arguments(i), NONE);
   }
   array.length = length;
   return array;
 }
 
 // -------------------------------------------------------------------
 
 function HarmonyArrayExtendArrayPrototype() {
   %CheckIsBootstrapping();
 
+  %FunctionSetLength(ArrayFrom, 1);
+
   // Set up non-enumerable functions on the Array object.
   InstallFunctions($Array, DONT_ENUM, $Array(
+    "from", ArrayFrom,
     "of", ArrayOf
   ));
 
   // Set up the non-enumerable functions on the Array prototype object.
   InstallFunctions($Array.prototype, DONT_ENUM, $Array(
     "find", ArrayFind,
     "findIndex", ArrayFindIndex,
     "fill", ArrayFill
   ));
 }
 
 HarmonyArrayExtendArrayPrototype();