Implement subclassing Arrays.
authordslomov <dslomov@chromium.org>
Tue, 3 Mar 2015 13:40:29 +0000 (05:40 -0800)
committerCommit bot <commit-bot@chromium.org>
Tue, 3 Mar 2015 13:40:39 +0000 (13:40 +0000)
R=mvstanton@chromium.org,arv@chromium.org,rossberg@chromium.org
BUG=v8:3930
LOG=Y

Review URL: https://codereview.chromium.org/975463002

Cr-Commit-Position: refs/heads/master@{#26960}

src/arm/code-stubs-arm.cc
src/arm64/code-stubs-arm64.cc
src/ia32/code-stubs-ia32.cc
src/mips/code-stubs-mips.cc
src/mips64/code-stubs-mips64.cc
src/runtime/runtime-array.cc
src/runtime/runtime.h
src/x64/code-stubs-x64.cc
test/mjsunit/harmony/classes-subclass-arrays.js [new file with mode: 0644]
test/mjsunit/harmony/regress/regress-3930.js [deleted file]

index 6ceeb03..68e430d 100644 (file)
@@ -4639,7 +4639,25 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
   __ bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ push(r1);
+  __ push(r3);
+
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ add(r0, r0, Operand(2));
+      break;
+    case NONE:
+      __ mov(r0, Operand(2));
+      break;
+    case ONE:
+      __ mov(r0, Operand(3));
+      break;
+  }
+
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()));
 }
 
 
index a3fd1a4..1a8cbea 100644 (file)
@@ -5016,12 +5016,11 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub(
 void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   ASM_LOCATION("ArrayConstructorStub::Generate");
   // ----------- S t a t e -------------
-  //  -- x0 : argc (only if argument_count() == ANY)
+  //  -- x0 : argc (only if argument_count() is ANY or MORE_THAN_ONE)
   //  -- x1 : constructor
   //  -- x2 : AllocationSite or undefined
   //  -- x3 : original constructor
-  //  -- sp[0] : return address
-  //  -- sp[4] : last argument
+  //  -- sp[0] : last argument
   // -----------------------------------
   Register constructor = x1;
   Register allocation_site = x2;
@@ -5065,8 +5064,24 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ Bind(&no_info);
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
+  // Subclassing support.
   __ Bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ Push(constructor, original_constructor);
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ add(x0, x0, Operand(2));
+      break;
+    case NONE:
+      __ Mov(x0, Operand(2));
+      break;
+    case ONE:
+      __ Mov(x0, Operand(3));
+      break;
+  }
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()));
 }
 
 
index 93cd7d9..f9bc28d 100644 (file)
@@ -4646,7 +4646,7 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub(
 
 void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   // ----------- S t a t e -------------
-  //  -- eax : argc (only if argument_count() == ANY)
+  //  -- eax : argc (only if argument_count() is ANY or MORE_THAN_ONE)
   //  -- ebx : AllocationSite or undefined
   //  -- edi : constructor
   //  -- edx : Original constructor
@@ -4680,9 +4680,6 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ cmp(ebx, isolate()->factory()->undefined_value());
   __ j(equal, &no_info);
 
-  __ cmp(edx, edi);
-  __ j(not_equal, &subclassing);
-
   // Only look at the lower 16 bits of the transition info.
   __ mov(edx, FieldOperand(ebx, AllocationSite::kTransitionInfoOffset));
   __ SmiUntag(edx);
@@ -4693,8 +4690,29 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ bind(&no_info);
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
+  // Subclassing.
   __ bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ pop(ecx);  // return address.
+  __ push(edi);
+  __ push(edx);
+
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ add(eax, Immediate(2));
+      break;
+    case NONE:
+      __ mov(eax, Immediate(2));
+      break;
+    case ONE:
+      __ mov(eax, Immediate(3));
+      break;
+  }
+
+  __ push(ecx);
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()));
 }
 
 
index 7954699..c4fc383 100644 (file)
@@ -4797,12 +4797,11 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub(
 
 void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   // ----------- S t a t e -------------
-  //  -- a0 : argc (only if argument_count() == ANY)
+  //  -- a0 : argc (only if argument_count() is ANY or MORE_THAN_ONE)
   //  -- a1 : constructor
   //  -- a2 : AllocationSite or undefined
   //  -- a3 : Original constructor
-  //  -- sp[0] : return address
-  //  -- sp[4] : last argument
+  //  -- sp[0] : last argument
   // -----------------------------------
 
   if (FLAG_debug_code) {
@@ -4840,8 +4839,28 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ bind(&no_info);
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
+  // Subclassing.
   __ bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ Push(a1);
+  __ Push(a3);
+
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ li(at, Operand(2));
+      __ addu(a0, a0, at);
+      break;
+    case NONE:
+      __ li(a0, Operand(2));
+      break;
+    case ONE:
+      __ li(a0, Operand(3));
+      break;
+  }
+
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()));
 }
 
 
index 5509dde..8e9fd03 100644 (file)
@@ -4844,8 +4844,7 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   //  -- a1 : constructor
   //  -- a2 : AllocationSite or undefined
   //  -- a3 : original constructor
-  //  -- sp[0] : return address
-  //  -- sp[4] : last argument
+  //  -- sp[0] : last argument
   // -----------------------------------
 
   if (FLAG_debug_code) {
@@ -4883,8 +4882,28 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ bind(&no_info);
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
+  // Subclassing.
   __ bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ Push(a1);
+  __ Push(a3);
+
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ li(at, Operand(2));
+      __ addu(a0, a0, at);
+      break;
+    case NONE:
+      __ li(a0, Operand(2));
+      break;
+    case ONE:
+      __ li(a0, Operand(3));
+      break;
+  }
+
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()));
 }
 
 
index 6d8a11e..f310309 100644 (file)
@@ -1038,6 +1038,7 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) {
 
 static Object* ArrayConstructorCommon(Isolate* isolate,
                                       Handle<JSFunction> constructor,
+                                      Handle<JSFunction> original_constructor,
                                       Handle<AllocationSite> site,
                                       Arguments* caller_args) {
   Factory* factory = isolate->factory();
@@ -1109,6 +1110,19 @@ static Object* ArrayConstructorCommon(Isolate* isolate,
     // We must mark the allocationsite as un-inlinable.
     site->SetDoNotInlineCall();
   }
+
+  // Set up the prototoype using original function.
+  // TODO(dslomov): instead of setting the __proto__,
+  // use and cache the correct map.
+  if (*original_constructor != *constructor) {
+    if (original_constructor->has_instance_prototype()) {
+      Handle<Object> prototype =
+          handle(original_constructor->instance_prototype(), isolate);
+      RETURN_FAILURE_ON_EXCEPTION(
+          isolate, JSObject::SetPrototype(array, prototype, false));
+    }
+  }
+
   return *array;
 }
 
@@ -1142,7 +1156,19 @@ RUNTIME_FUNCTION(Runtime_ArrayConstructor) {
     DCHECK(!site->SitePointsToLiteral());
   }
 
-  return ArrayConstructorCommon(isolate, constructor, site, caller_args);
+  return ArrayConstructorCommon(isolate, constructor, constructor, site,
+                                caller_args);
+}
+
+
+RUNTIME_FUNCTION(Runtime_ArrayConstructorWithSubclassing) {
+  HandleScope scope(isolate);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, constructor, args.length() - 2);
+  CONVERT_ARG_HANDLE_CHECKED(JSFunction, original_constructor,
+                             args.length() - 1);
+  Arguments caller_args(args.length() - 2, args.arguments());
+  return ArrayConstructorCommon(isolate, constructor, original_constructor,
+                                Handle<AllocationSite>::null(), &caller_args);
 }
 
 
@@ -1161,7 +1187,7 @@ RUNTIME_FUNCTION(Runtime_InternalArrayConstructor) {
     DCHECK(arg_count == caller_args->length());
   }
 #endif
-  return ArrayConstructorCommon(isolate, constructor,
+  return ArrayConstructorCommon(isolate, constructor, constructor,
                                 Handle<AllocationSite>::null(), caller_args);
 }
 
index 57d450d..f173cc9 100644 (file)
@@ -465,6 +465,7 @@ namespace internal {
                                                              \
   /* Arrays */                                               \
   F(ArrayConstructor, -1, 1)                                 \
+  F(ArrayConstructorWithSubclassing, -1, 1)                  \
   F(InternalArrayConstructor, -1, 1)                         \
                                                              \
   /* Literals */                                             \
index cc92714..dd79a4e 100644 (file)
@@ -4636,8 +4636,30 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) {
   __ bind(&no_info);
   GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES);
 
+  // Subclassing
   __ bind(&subclassing);
-  __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1);
+  __ Pop(rcx);  // return address.
+  __ Push(rdi);
+  __ Push(rdx);
+
+  // Adjust argc.
+  switch (argument_count()) {
+    case ANY:
+    case MORE_THAN_ONE:
+      __ addp(rax, Immediate(2));
+      break;
+    case NONE:
+      __ movp(rax, Immediate(2));
+      break;
+    case ONE:
+      __ movp(rax, Immediate(3));
+      break;
+  }
+
+  __ Push(rcx);
+  __ JumpToExternalReference(
+      ExternalReference(Runtime::kArrayConstructorWithSubclassing, isolate()),
+      1);
 }
 
 
diff --git a/test/mjsunit/harmony/classes-subclass-arrays.js b/test/mjsunit/harmony/classes-subclass-arrays.js
new file mode 100644 (file)
index 0000000..e0363c7
--- /dev/null
@@ -0,0 +1,150 @@
+// Copyright 2015 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.
+//
+// Flags: --harmony-classes
+'use strict';
+
+(function TestDefaultConstructor() {
+  class Stack extends Array { }
+  {
+    let s1 = new Stack();
+    assertSame(Stack.prototype, s1.__proto__);
+    assertTrue(Array.isArray(s1));
+    assertSame(0, s1.length);
+    s1[0] = 'xyz';
+    assertSame(1, s1.length);
+    assertSame('xyz', s1[0]);
+    s1.push(42);
+    assertSame(2, s1.length);
+    assertSame('xyz', s1[0]);
+    assertSame(42, s1[1]);
+  }
+
+  {
+    let s2 = new Stack(10);
+    assertSame(Stack.prototype, s2.__proto__);
+    assertTrue(Array.isArray(s2));
+    assertSame(10, s2.length);
+    assertSame(undefined, s2[0]);
+  }
+
+  {
+    let a = [1,2,3];
+    let s3 = new Stack(a);
+    assertSame(Stack.prototype, s3.__proto__);
+    assertTrue(Array.isArray(s3));
+    assertSame(1, s3.length);
+    assertSame(a, s3[0]);
+  }
+
+  {
+    let s4 = new Stack(1, 2, 3);
+    assertSame(Stack.prototype, s4.__proto__);
+    assertTrue(Array.isArray(s4));
+    assertSame(3, s4.length);
+    assertSame(1, s4[0]);
+    assertSame(2, s4[1]);
+    assertSame(3, s4[2]);
+  }
+
+  {
+    let s5 = new Stack(undefined, undefined, undefined);
+    assertSame(Stack.prototype, s5.__proto__);
+    assertTrue(Array.isArray(s5));
+    assertSame(3, s5.length);
+    assertSame(undefined, s5[0]);
+    assertSame(undefined, s5[1]);
+    assertSame(undefined, s5[2]);
+  }
+}());
+
+
+(function TestEmptyArgsSuper() {
+  class Stack extends Array {
+    constructor() { super(); }
+  }
+  let s1 = new Stack();
+  assertSame(Stack.prototype, s1.__proto__);
+  assertTrue(Array.isArray(s1));
+  assertSame(0, s1.length);
+  s1[0] = 'xyz';
+  assertSame(1, s1.length);
+  assertSame('xyz', s1[0]);
+  s1.push(42);
+  assertSame(2, s1.length);
+  assertSame('xyz', s1[0]);
+  assertSame(42, s1[1]);
+}());
+
+
+(function TestOneArgSuper() {
+  class Stack extends Array {
+    constructor(x) {
+      super(x);
+    }
+  }
+
+  {
+    let s2 = new Stack(10, 'ignored arg');
+    assertSame(Stack.prototype, s2.__proto__);
+    assertTrue(Array.isArray(s2));
+    assertSame(10, s2.length);
+    assertSame(undefined, s2[0]);
+  }
+
+  {
+    let a = [1,2,3];
+    let s3 = new Stack(a, 'ignored arg');
+    assertSame(Stack.prototype, s3.__proto__);
+    assertTrue(Array.isArray(s3));
+    assertSame(1, s3.length);
+    assertSame(a, s3[0]);
+  }
+}());
+
+
+(function TestMultipleArgsSuper() {
+  class Stack extends Array {
+    constructor(x, y, z) {
+      super(x, y, z);
+    }
+  }
+  {
+    let s4 = new Stack(1, 2, 3, 4, 5);
+    assertSame(Stack.prototype, s4.__proto__);
+    assertTrue(Array.isArray(s4));
+    assertSame(3, s4.length);
+    assertSame(1, s4[0]);
+    assertSame(2, s4[1]);
+    assertSame(3, s4[2]);
+  }
+
+  {
+    let s5 = new Stack(undefined);
+    assertSame(Stack.prototype, s5.__proto__);
+    assertTrue(Array.isArray(s5));
+    assertTrue(s5.__proto__ == Stack.prototype);
+    assertSame(3, s5.length);
+    assertSame(undefined, s5[0]);
+    assertSame(undefined, s5[1]);
+    assertSame(undefined, s5[2]);
+  }
+}());
+
+
+(function TestArrayConcat() {
+  class Stack extends Array { }
+  let s1 = new Stack(1,2,3);
+
+  assertArrayEquals([1,2,3,4,5,6], s1.concat([4,5,6]));
+  assertArrayEquals([4,5,6,1,2,3], [4,5,6].concat(s1));
+}());
+
+
+(function TestJSONStringify() {
+  class Stack extends Array { }
+
+  let s1 = new Stack(1,2,3);
+  assertSame("[1,2,3]", JSON.stringify(s1));
+}());
diff --git a/test/mjsunit/harmony/regress/regress-3930.js b/test/mjsunit/harmony/regress/regress-3930.js
deleted file mode 100644 (file)
index 2436a2d..0000000
+++ /dev/null
@@ -1,28 +0,0 @@
-// Copyright 2015 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.
-//
-// Flags: --harmony-classes
-'use strict';
-
-class Stack extends Array { }
-
-assertThrows(function() { new Stack(); }, TypeError);
-
-class Stack1 extends Array {
-  constructor() { super(); }
-}
-
-assertThrows(function() { new Stack1(); }, TypeError);
-
-class Stack2 extends Array {
-  constructor() { super(1, 25); }
-}
-
-assertThrows(function() { new Stack2(); }, TypeError);
-
-let X = Array;
-
-class Stack4 extends X { }
-
-assertThrows(function() { new Stack2(); }, TypeError);