Implement Array.from()

src/harmony-array.js

Index: src/harmony-array.js
diff --git a/src/harmony-array.js b/src/harmony-array.js
index 88b878f0a76ef594f64f7905d61459c05749724b..e3db8322252e39407bf3cec802911a5557498472 100644
--- a/src/harmony-array.js
+++ b/src/harmony-array.js
@@ -123,6 +123,67 @@ function ArrayFill(value /* [, start [, end ] ] */) {  // length == 1
   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();
@@ -141,8 +202,11 @@ function ArrayOf() {
 function HarmonyArrayExtendArrayPrototype() {
   %CheckIsBootstrapping();
 
+  %FunctionSetLength(ArrayFrom, 1);
+
   // Set up non-enumerable functions on the Array object.
   InstallFunctions($Array, DONT_ENUM, $Array(
+    "from", ArrayFrom,
     "of", ArrayOf
   ));