Use ES2015-style TypedArray prototype chain

src/js/typedarray.js

Index: src/js/typedarray.js
diff --git a/src/js/typedarray.js b/src/js/typedarray.js
index 2c0e0145bb5ca0034f4b3c3aefb30a5d7695effa..ef978c9cd9d60edf777093420b8b46c7ce5d92c9 100644
--- a/src/js/typedarray.js
+++ b/src/js/typedarray.js
@@ -224,38 +224,7 @@ function NAMEConstructor(arg1, arg2, arg3) {
   }
 }
 
-function NAME_GetBuffer() {
-  if (!(%_ClassOf(this) === 'NAME')) {
-    throw MakeTypeError(kIncompatibleMethodReceiver, "NAME.buffer", this);
-  }
-  return %TypedArrayGetBuffer(this);
-}
-
-function NAME_GetByteLength() {
-  if (!(%_ClassOf(this) === 'NAME')) {
-    throw MakeTypeError(kIncompatibleMethodReceiver, "NAME.byteLength", this);
-  }
-  return %_ArrayBufferViewGetByteLength(this);
-}
-
-function NAME_GetByteOffset() {
-  if (!(%_ClassOf(this) === 'NAME')) {
-    throw MakeTypeError(kIncompatibleMethodReceiver, "NAME.byteOffset", this);
-  }
-  return %_ArrayBufferViewGetByteOffset(this);
-}
-
-function NAME_GetLength() {
-  if (!(%_ClassOf(this) === 'NAME')) {
-    throw MakeTypeError(kIncompatibleMethodReceiver, "NAME.length", this);
-  }
-  return %_TypedArrayGetLength(this);
-}
-
 function NAMESubArray(begin, end) {
-  if (!(%_ClassOf(this) === 'NAME')) {
-    throw MakeTypeError(kIncompatibleMethodReceiver, "NAME.subarray", this);
-  }
   var beginInt = TO_INTEGER(begin);
   if (!IS_UNDEFINED(end)) {
     var endInt = TO_INTEGER(end);
@@ -291,6 +260,56 @@ endmacro
 
 TYPED_ARRAYS(TYPED_ARRAY_CONSTRUCTOR)
 
+function TypedArraySubArray(begin, end) {
+  switch (%_ClassOf(this)) {
+macro TYPED_ARRAY_SUBARRAY_CASE(ARRAY_ID, NAME, ELEMENT_SIZE)
+    case "NAME":
+      return %_Call(NAMESubArray, this, begin, end);
+endmacro
+TYPED_ARRAYS(TYPED_ARRAY_SUBARRAY_CASE)

[Camillo Bruni, 2015/12/23 15:56:02]

Now that we have a common prototype we could implement the SubArray helper
directly here (the ELEMENT_SIZE and GlobalNAME are the two different args that
could be stored in a local variable here.

[Dan Ehrenberg, 2015/12/23 23:14:46]

I was initially doing that, but I actually wanted the switch statement to be
inlined and the things it calls out to to be out-of-line. Of course, decisions
like these should be validated with benchmarks. I reached out to a member of the
Tactile team for this, but haven't heard back yet. I can wait to land this until
I there's more of that kind of evidence, if you prefer.

+  }
+  throw MakeTypeError(kIncompatibleMethodReceiver,
+                      "get TypedArray.prototype.subarray", this);
+}
+%SetForceInlineFlag(TypedArraySubArray);
+
+function TypedArrayGetBuffer() {
+  if (!%_IsTypedArray(this)) {
+    throw MakeTypeError(kIncompatibleMethodReceiver,
+                        "get TypedArray.prototype.buffer", this);
+  }
+  return %TypedArrayGetBuffer(this);
+}
+%SetForceInlineFlag(TypedArrayGetBuffer);
+
+function TypedArrayGetByteLength() {
+  if (!%_IsTypedArray(this)) {
+    throw MakeTypeError(kIncompatibleMethodReceiver,
+                        "get TypedArray.prototype.byteLength", this);
+  }
+  return %_ArrayBufferViewGetByteLength(this);
+}
+%SetForceInlineFlag(TypedArrayGetByteLength);
+
+function TypedArrayGetByteOffset() {
+  if (!%_IsTypedArray(this)) {
+    throw MakeTypeError(kIncompatibleMethodReceiver,
+                        "get TypedArray.prototype.byteOffset", this);
+  }
+  return %_ArrayBufferViewGetByteOffset(this);
+}
+%SetForceInlineFlag(TypedArrayGetByteOffset);
+
+function TypedArrayGetLength() {
+  if (!%_IsTypedArray(this)) {
+    throw MakeTypeError(kIncompatibleMethodReceiver,
+                        "get TypedArray.prototype.length", this);
+  }
+  return %_TypedArrayGetLength(this);
+}
+%SetForceInlineFlag(TypedArrayGetLength);
+
+
 
 function TypedArraySetFromArrayLike(target, source, sourceLength, offset) {
   if (offset > 0) {
@@ -719,59 +738,73 @@ function TypedArrayFrom(source, mapfn, thisArg) {
 }
 %FunctionSetLength(TypedArrayFrom, 1);
 
+function TypedArray() {
+  if (IS_UNDEFINED(new.target)) {
+    throw MakeTypeError(kConstructorNonCallable, "TypedArray");
+  }
+  if (new.target === TypedArray) {

[caitp (gmail), 2015/12/22 23:47:45]

question: how is this supposed to work if new.target is TypedArray from a
different context, with __proto__ set to this context's TypedArray? It doesn't
look like it would throw in that case, and could possibly cause some assertions
to fail when methods are invoked on it. I'm not sure

[Dan Ehrenberg, 2015/12/22 23:53:08]

Well, this definitely implements the spec in this particular respect. TypedArray
will refer to that same function object that is currently executing. I argued
that this test is not meaningful to have on GitHub, but I ended up going with
Allen's suggestion to include the check.

[caitp (gmail), 2015/12/22 23:56:28]

It looks spec compliant --- but I'm not sure if it necessarily interacts well
with the rest of the TypedArray implementation in V8. I might be overthinking
it, but maybe a cctest with mixesd contexts + TypedArray method invocations
might be helpful

[Camillo Bruni, 2015/12/23 15:56:02]

Just use Realms, it's quite easy (see for instance
proxies-cross-realm-exception.js).

[Dan Ehrenberg, 2015/12/23 23:14:46]

Well, to write a test I'll have to understand more about the potential issue. I
still don't really see what could go wrong here. But thanks for the
reference--that'll be really useful for when I'm testing @@species.

+    throw MakeTypeError(kConstructAbstractClass, "TypedArray");
+  }
+}
+
 // -------------------------------------------------------------------
 
-// TODO(littledan): Fix the TypedArray proto chain (bug v8:4085).
+%FunctionSetPrototype(TypedArray, new GlobalObject());

[Camillo Bruni, 2015/12/23 15:56:02]

According to the Spec 22.2.2.3 the prototype should be read-only, but I think in
your case it's configurable (probably you have to create TypedArray in
bootstrapper.cc)

[Dan Ehrenberg, 2015/12/23 23:14:45]

It seems to be nonconfigurable, nonwritable, nonenumerable, as the spec
requires. I added a test for this. I'm having trouble finding out how this works
exactly, though...

[Camillo Bruni, 2015/12/28 08:54:45]

ah ok then :) I thought that this would only be the case for things set up in
bootstrapper.cc.

+%AddNamedProperty(TypedArray.prototype,
+                  "constructor", TypedArray, DONT_ENUM);
+utils.InstallFunctions(TypedArray, DONT_ENUM | DONT_DELETE | READ_ONLY, [
+  "from", TypedArrayFrom,
+  "of", TypedArrayOf
+]);
+utils.InstallGetter(TypedArray.prototype, "buffer", TypedArrayGetBuffer);
+utils.InstallGetter(TypedArray.prototype, "byteOffset", TypedArrayGetByteOffset,
+                    DONT_ENUM | DONT_DELETE);
+utils.InstallGetter(TypedArray.prototype, "byteLength",
+                    TypedArrayGetByteLength, DONT_ENUM | DONT_DELETE);
+utils.InstallGetter(TypedArray.prototype, "length", TypedArrayGetLength,
+                    DONT_ENUM | DONT_DELETE);
+utils.InstallGetter(TypedArray.prototype, toStringTagSymbol,
+                    TypedArrayGetToStringTag);
+utils.InstallFunctions(TypedArray.prototype, DONT_ENUM, [
+  "subarray", TypedArraySubArray,
+  "set", TypedArraySet,
+  "copyWithin", TypedArrayCopyWithin,
+  "every", TypedArrayEvery,
+  "fill", TypedArrayFill,
+  "filter", TypedArrayFilter,
+  "find", TypedArrayFind,
+  "findIndex", TypedArrayFindIndex,
+  "includes", TypedArrayIncludes,
+  "indexOf", TypedArrayIndexOf,
+  "join", TypedArrayJoin,
+  "lastIndexOf", TypedArrayLastIndexOf,
+  "forEach", TypedArrayForEach,
+  "map", TypedArrayMap,
+  "reduce", TypedArrayReduce,
+  "reduceRight", TypedArrayReduceRight,
+  "reverse", TypedArrayReverse,
+  "slice", TypedArraySlice,
+  "some", TypedArraySome,
+  "sort", TypedArraySort,
+  "toString", TypedArrayToString,
+  "toLocaleString", TypedArrayToLocaleString
+]);
+
+
 macro SETUP_TYPED_ARRAY(ARRAY_ID, NAME, ELEMENT_SIZE)
   %SetCode(GlobalNAME, NAMEConstructor);
   %FunctionSetPrototype(GlobalNAME, new GlobalObject());
+  %InternalSetPrototype(GlobalNAME, TypedArray);
+  %InternalSetPrototype(GlobalNAME.prototype, TypedArray.prototype);
 
   %AddNamedProperty(GlobalNAME, "BYTES_PER_ELEMENT", ELEMENT_SIZE,
                     READ_ONLY | DONT_ENUM | DONT_DELETE);
 
-  utils.InstallFunctions(GlobalNAME, DONT_ENUM | DONT_DELETE | READ_ONLY, [
-    "from", TypedArrayFrom,
-    "of", TypedArrayOf
-  ]);
-
   %AddNamedProperty(GlobalNAME.prototype,
                     "constructor", global.NAME, DONT_ENUM);
   %AddNamedProperty(GlobalNAME.prototype,
                     "BYTES_PER_ELEMENT", ELEMENT_SIZE,
                     READ_ONLY | DONT_ENUM | DONT_DELETE);
-  utils.InstallGetter(GlobalNAME.prototype, "buffer", NAME_GetBuffer);
-  utils.InstallGetter(GlobalNAME.prototype, "byteOffset", NAME_GetByteOffset,
-                      DONT_ENUM | DONT_DELETE);
-  utils.InstallGetter(GlobalNAME.prototype, "byteLength", NAME_GetByteLength,
-                      DONT_ENUM | DONT_DELETE);
-  utils.InstallGetter(GlobalNAME.prototype, "length", NAME_GetLength,
-                      DONT_ENUM | DONT_DELETE);
-  utils.InstallGetter(GlobalNAME.prototype, toStringTagSymbol,
-                      TypedArrayGetToStringTag);
-  utils.InstallFunctions(GlobalNAME.prototype, DONT_ENUM, [
-    "subarray", NAMESubArray,
-    "set", TypedArraySet,
-    "copyWithin", TypedArrayCopyWithin,
-    "every", TypedArrayEvery,
-    "fill", TypedArrayFill,
-    "filter", TypedArrayFilter,
-    "find", TypedArrayFind,
-    "findIndex", TypedArrayFindIndex,
-    "includes", TypedArrayIncludes,
-    "indexOf", TypedArrayIndexOf,
-    "join", TypedArrayJoin,
-    "lastIndexOf", TypedArrayLastIndexOf,
-    "forEach", TypedArrayForEach,
-    "map", TypedArrayMap,
-    "reduce", TypedArrayReduce,
-    "reduceRight", TypedArrayReduceRight,
-    "reverse", TypedArrayReverse,
-    "slice", TypedArraySlice,
-    "some", TypedArraySome,
-    "sort", TypedArraySort,
-    "toString", TypedArrayToString,
-    "toLocaleString", TypedArrayToLocaleString
-  ]);
 endmacro
 
 TYPED_ARRAYS(SETUP_TYPED_ARRAY)