Migrate %TypedArray%.prototype.fill to C++

src/builtins/builtins-typedarray.cc

Index: src/builtins/builtins-typedarray.cc
diff --git a/src/builtins/builtins-typedarray.cc b/src/builtins/builtins-typedarray.cc
index 8a8bde4be027a1329ccaaea4dfe92d75c32957a3..a44ea957780c4cce1bf379f6267370c8922ea877 100644
--- a/src/builtins/builtins-typedarray.cc
+++ b/src/builtins/builtins-typedarray.cc
@@ -456,5 +456,79 @@ BUILTIN(TypedArrayPrototypeCopyWithin) {
   return *array;
 }
 
+BUILTIN(TypedArrayPrototypeFill) {
+  HandleScope scope(isolate);
+
+  Handle<JSTypedArray> array;
+  const char* method = "%TypedArray%.prototype.fill";
+  ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+      isolate, array, JSTypedArray::Validate(isolate, args.receiver(), method));
+
+  if (V8_UNLIKELY(array->WasNeutered())) return *array;

[Camillo Bruni, 2017/03/06 09:13:44]

This check has to happen after all the argument conversions are finished!
If you follow the ToInteger spec steps (this is really well hidden :P) you see
that it can have side-effects and for instance neuter the array.

[rongjie, 2017/03/06 10:35:49]

Done.

+
+  int64_t len = array->length_value();
+  int64_t start = 0;
+  int64_t end = len;
+  double fp = 0.0;

[Camillo Bruni, 2017/03/06 09:13:44]

nit: please use proper variable names, fp is kind of an overloaded term ;) (And
I know that the spec likes to use single character variable names :/)

[rongjie, 2017/03/06 10:35:49]

Done.

+
+  if (V8_LIKELY(args.length() > 1)) {
+    Handle<Object> num;
+    ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+        isolate, num, Object::ToNumber(args.at<Object>(1)));

[Camillo Bruni, 2017/03/06 09:13:44]

You cannot simply do a ToNumber on the value input parameter.
Sadly the spec is written in weird way here, forcing us to to a ToNumber on
every! loop iteration.
- https://tc39.github.io/ecma262/#sec-array.prototype.fill which calls Set(...)
-
https://tc39.github.io/ecma262/#sec-integer-indexed-exotic-objects-set-p-v-re...
- Which ends up here:
https://tc39.github.io/ecma262/#sec-integerindexedelementset where the step 3 is
ToNumber

So you have to probably bail out if the value is not a primitive value (IsNumber
probably good enough). In all other cases ToNumber has a side-effect via
ToPrimitive which could call the @@toPrimitive method *sigh* :).

My suggestion, implement a runtime function which does the very slow spec
conversion steps.

+
+    if (num->IsSmi()) {
+      fp = Smi::cast(*num)->value();
+    } else {
+      DCHECK(num->IsHeapNumber());
+      fp = HeapNumber::cast(*num)->value();
+    }
+    if (array->type() == kExternalUint8ClampedArray) {
+      fp = std::min<double>(std::max<double>(0, fp), 255);
+    }

[rongjie, 2017/03/04 07:24:46]

Casting integer to and from double might cause loss of precision?

+
+    Handle<Object> optional = args.atOrUndefined(isolate, 2);
+    if (!optional->IsUndefined(isolate)) {
+      ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+          isolate, num, Object::ToInteger(isolate, optional));
+      start = CapRelativeIndex(num, 0, len);
+
+      optional = args.atOrUndefined(isolate, 3);
+      if (!optional->IsUndefined(isolate)) {
+        ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
+            isolate, num, Object::ToInteger(isolate, optional));
+        end = CapRelativeIndex(num, 0, len);
+      }
+    }
+  }
+
+  int64_t count = end - start;
+  if (count <= 0) return *array;
+
+  if (V8_UNLIKELY(array->WasNeutered())) return *array;
+
+  // Ensure processed indexes are within array bounds
+  DCHECK_GE(start, 0);
+  DCHECK_LT(start, len);
+  DCHECK_GE(end, 0);
+  DCHECK_LE(end, len);
+  DCHECK_LE(count, len);
+
+  switch (array->type()) {
+#define TYPED_ARRAY_FILL(Type, type, TYPE, ctype, size)                \
+  case kExternal##Type##Array: {                                       \
+    ctype* backing_store =                                             \
+        static_cast<ctype*>(array->GetBuffer()->backing_store());      \
+    ctype cast_value = static_cast<ctype>(fp);                         \
+    std::fill(backing_store + start, backing_store + end, cast_value); \
+    break;                                                             \
+  }
+
+    TYPED_ARRAYS(TYPED_ARRAY_FILL)
+#undef TYPED_ARRAY_FILL
+  }
+
+  return *array;
+}
+
 }  // namespace internal
 }  // namespace v8