In Array.of and Array.from, fall back to DefineOwnProperty

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.
 
 (function(global, utils) {
 
 'use strict';
 
 %CheckIsBootstrapping();
 
 // -------------------------------------------------------------------
 // Imports
 
 var GlobalArray = global.Array;
 var GlobalSymbol = global.Symbol;
 
 var GetIterator;
 var GetMethod;
 var MathMax;
 var MathMin;
 var ObjectIsFrozen;
+var ObjectDefineProperty;
 
 utils.Import(function(from) {
   GetIterator = from.GetIterator;
   GetMethod = from.GetMethod;
   MathMax = from.MathMax;
   MathMin = from.MathMin;
   ObjectIsFrozen = from.ObjectIsFrozen;
+  ObjectDefineProperty = from.ObjectDefineProperty;
 });
 
 // -------------------------------------------------------------------
 
 function InnerArrayCopyWithin(target, start, end, array, length) {
   target = TO_INTEGER(target);
   var to;
   if (target < 0) {
     to = MathMax(length + target, 0);
   } else {
@@ skipping 145 matching lines @@
 // ES6, draft 04-05-14, section 22.1.3.6
 function ArrayFill(value, start, end) {
   CHECK_OBJECT_COERCIBLE(this, "Array.prototype.fill");
 
   var array = $toObject(this);
   var length = TO_UINT32(array.length);
 
   return InnerArrayFill(value, start, end, array, length);
 }
 
+function AddArrayElement(constructor, array, i, value) {
+  if (constructor === GlobalArray) {

[Toon Verwaest, 2015/06/11 08:03:14]

Is this enough? A constructor could return an array with predefined
getters/setters...

[arv (Not doing code reviews), 2015/06/11 13:04:01]

I believe this is fine.

constructor here is the receiver in Array.from. The array passed in here is a
new instance created from Array which will never have own accessors.

[Toon Verwaest, 2015/06/11 13:09:23]

(d8):1: illegal access

[adamk, 2015/06/11 15:12:10]

Do you have Dan's patch patched in? This should go down the slow path because f
!== GlobalArray.

[arv (Not doing code reviews), 2015/06/11 15:19:37]

Once we add support for @@species then we will have to use the slow path again.
An assert would have been good.

[adamk, 2015/06/11 15:26:40]

Not sure I follow, Array.from/of only look at the receiver, they don't check
@@species.

[arv (Not doing code reviews), 2015/06/11 15:30:01]

You are right. nm.

[Toon Verwaest, 2015/06/11 15:35:33]

Arg, ok, I see. Nevermind ;)

However, shouldn't we make sure that if this is not a constructor, we also pass
in GlobalArray?

[Dan Ehrenberg, 2015/06/11 16:27:25]

I think GlobalArray will remain a constructor, so we'll go down the else path if
a non-constructor is passed in.

+    %AddElement(array, i, value, NONE);
+  } else {
+    ObjectDefineProperty(array, i, { value: value });

[arv (Not doing code reviews), 2015/06/11 13:04:01]

non enum? non writable? non configurable?

We really should expose something faster than using this high level API that
uses property descriptors.

How about we provide a function called CreateDataPropertyOrThrow or
CreateDataProperty?

We could also skip the descriptor and use the bit mask we are using internally
(NONE, DONT_ENUM etc)

The refactoring can be done later but the descriptor (enum, writable,
configurable) needs to be fixed and tested.

[Dan Ehrenberg, 2015/06/11 16:27:25]

Fixed the descriptor and added tests.

+  }
+}
+
 // ES6, draft 10-14-14, section 22.1.2.1
 function ArrayFrom(arrayLike, mapfn, receiver) {
   var items = $toObject(arrayLike);
   var mapping = !IS_UNDEFINED(mapfn);
 
   if (mapping) {
     if (!IS_SPEC_FUNCTION(mapfn)) {
       throw MakeTypeError(kCalledNonCallable, mapfn);
     } else if (%IsSloppyModeFunction(mapfn)) {
       if (IS_NULL(receiver)) {
@@ skipping 27 matching lines @@
         result.length = k;
         return result;
       }
 
       nextValue = next.value;
       if (mapping) {
         mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
       } else {
         mappedValue = nextValue;
       }
-      %AddElement(result, k++, mappedValue, NONE);
+      AddArrayElement(this, result, k, mappedValue);
+      k++;
     }
   } else {
     var len = $toLength(items.length);
     result = %IsConstructor(this) ? new this(len) : new GlobalArray(len);
 
     for (k = 0; k < len; ++k) {
       nextValue = items[k];
       if (mapping) {
         mappedValue = %_CallFunction(receiver, nextValue, k, mapfn);
       } else {
         mappedValue = nextValue;
       }
-      %AddElement(result, k, mappedValue, NONE);
+      AddArrayElement(this, result, k, mappedValue);
     }
 
     result.length = k;
     return result;
   }
 }
 
 // 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 = %IsConstructor(constructor) ? new constructor(length) : [];
   for (var i = 0; i < length; i++) {
-    %AddElement(array, i, %_Arguments(i), NONE);
+    AddArrayElement(constructor, array, i, %_Arguments(i));
   }
   array.length = length;
   return array;
 }
 
 // -------------------------------------------------------------------
 
 utils.InstallConstants(GlobalSymbol, [
   // TODO(dslomov, caitp): Move to symbol.js when shipping
   "isConcatSpreadable", symbolIsConcatSpreadable
@@ skipping 24 matching lines @@
 
 utils.Export(function(to) {
   to.ArrayFrom = ArrayFrom;
   to.InnerArrayCopyWithin = InnerArrayCopyWithin;
   to.InnerArrayFill = InnerArrayFill;
   to.InnerArrayFind = InnerArrayFind;
   to.InnerArrayFindIndex = InnerArrayFindIndex;
 });
 
 })