From 7bf9ef1e397267978fbc78dfd59421d10e0716be Mon Sep 17 00:00:00 2001 From: dslomov Date: Tue, 3 Mar 2015 08:41:44 -0800 Subject: [PATCH] Revert of Implement subclassing Arrays. (patchset #7 id:110001 of https://codereview.chromium.org/975463002/) Reason for revert: Arm buildre complains again (why v8_linux_arm_dbg does not complain?) Original issue's description: > Implement subclassing Arrays. > > R=mvstanton@chromium.org,arv@chromium.org,rossberg@chromium.org > BUG=v8:3930 > LOG=Y > > Committed: https://crrev.com/6898da1a28d64d1fb2962804ba566f6d618ffc70 > Cr-Commit-Position: refs/heads/master@{#26960} > > Committed: https://crrev.com/8d29cc11a56e77297792fe100986a80b65de0051 > Cr-Commit-Position: refs/heads/master@{#26963} TBR=arv@chromium.org,mvstanton@chromium.org,rossberg@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:3930 Review URL: https://codereview.chromium.org/974963002 Cr-Commit-Position: refs/heads/master@{#26965} --- src/arm/code-stubs-arm.cc | 20 +-- src/arm64/code-stubs-arm64.cc | 23 +-- src/ia32/code-stubs-ia32.cc | 28 +--- src/mips/code-stubs-mips.cc | 27 +--- src/mips64/code-stubs-mips64.cc | 25 +-- src/runtime/runtime-array.cc | 31 +--- src/runtime/runtime.h | 1 - src/x64/code-stubs-x64.cc | 24 +-- .../harmony/classes-subclass-arrays.js | 150 ------------------ test/mjsunit/harmony/regress/regress-3930.js | 28 ++++ 10 files changed, 48 insertions(+), 309 deletions(-) delete mode 100644 test/mjsunit/harmony/classes-subclass-arrays.js create mode 100644 test/mjsunit/harmony/regress/regress-3930.js diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc index 68e430d29ba..6ceeb0363d8 100644 --- a/src/arm/code-stubs-arm.cc +++ b/src/arm/code-stubs-arm.cc @@ -4639,25 +4639,7 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); __ bind(&subclassing); - __ 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())); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/src/arm64/code-stubs-arm64.cc b/src/arm64/code-stubs-arm64.cc index 1a8cbea5129..a3fd1a4ca83 100644 --- a/src/arm64/code-stubs-arm64.cc +++ b/src/arm64/code-stubs-arm64.cc @@ -5016,11 +5016,12 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub( void ArrayConstructorStub::Generate(MacroAssembler* masm) { ASM_LOCATION("ArrayConstructorStub::Generate"); // ----------- S t a t e ------------- - // -- x0 : argc (only if argument_count() is ANY or MORE_THAN_ONE) + // -- x0 : argc (only if argument_count() == ANY) // -- x1 : constructor // -- x2 : AllocationSite or undefined // -- x3 : original constructor - // -- sp[0] : last argument + // -- sp[0] : return address + // -- sp[4] : last argument // ----------------------------------- Register constructor = x1; Register allocation_site = x2; @@ -5064,24 +5065,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { __ Bind(&no_info); GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); - // Subclassing support. __ Bind(&subclassing); - __ 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())); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc index f9bc28dd4bc..93cd7d9a320 100644 --- a/src/ia32/code-stubs-ia32.cc +++ b/src/ia32/code-stubs-ia32.cc @@ -4646,7 +4646,7 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub( void ArrayConstructorStub::Generate(MacroAssembler* masm) { // ----------- S t a t e ------------- - // -- eax : argc (only if argument_count() is ANY or MORE_THAN_ONE) + // -- eax : argc (only if argument_count() == ANY) // -- ebx : AllocationSite or undefined // -- edi : constructor // -- edx : Original constructor @@ -4680,6 +4680,9 @@ 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); @@ -4690,29 +4693,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { __ bind(&no_info); GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); - // Subclassing. __ bind(&subclassing); - __ 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())); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc index c4fc3835529..79546999837 100644 --- a/src/mips/code-stubs-mips.cc +++ b/src/mips/code-stubs-mips.cc @@ -4797,11 +4797,12 @@ void ArrayConstructorStub::GenerateDispatchToArrayStub( void ArrayConstructorStub::Generate(MacroAssembler* masm) { // ----------- S t a t e ------------- - // -- a0 : argc (only if argument_count() is ANY or MORE_THAN_ONE) + // -- a0 : argc (only if argument_count() == ANY) // -- a1 : constructor // -- a2 : AllocationSite or undefined // -- a3 : Original constructor - // -- sp[0] : last argument + // -- sp[0] : return address + // -- sp[4] : last argument // ----------------------------------- if (FLAG_debug_code) { @@ -4839,28 +4840,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { __ bind(&no_info); GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); - // Subclassing. __ bind(&subclassing); - __ 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())); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/src/mips64/code-stubs-mips64.cc b/src/mips64/code-stubs-mips64.cc index 8e9fd03fb00..5509dde33b9 100644 --- a/src/mips64/code-stubs-mips64.cc +++ b/src/mips64/code-stubs-mips64.cc @@ -4844,7 +4844,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { // -- a1 : constructor // -- a2 : AllocationSite or undefined // -- a3 : original constructor - // -- sp[0] : last argument + // -- sp[0] : return address + // -- sp[4] : last argument // ----------------------------------- if (FLAG_debug_code) { @@ -4882,28 +4883,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { __ bind(&no_info); GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); - // Subclassing. __ bind(&subclassing); - __ 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())); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/src/runtime/runtime-array.cc b/src/runtime/runtime-array.cc index 471a7a6adc3..6d8a11e2407 100644 --- a/src/runtime/runtime-array.cc +++ b/src/runtime/runtime-array.cc @@ -1038,7 +1038,6 @@ RUNTIME_FUNCTION(Runtime_GetArrayKeys) { static Object* ArrayConstructorCommon(Isolate* isolate, Handle constructor, - Handle original_constructor, Handle site, Arguments* caller_args) { Factory* factory = isolate->factory(); @@ -1110,19 +1109,6 @@ 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 prototype = - handle(original_constructor->instance_prototype(), isolate); - RETURN_FAILURE_ON_EXCEPTION( - isolate, JSObject::SetPrototype(array, prototype, false)); - } - } - return *array; } @@ -1156,20 +1142,7 @@ RUNTIME_FUNCTION(Runtime_ArrayConstructor) { DCHECK(!site->SitePointsToLiteral()); } - return ArrayConstructorCommon(isolate, constructor, constructor, site, - caller_args); -} - - -RUNTIME_FUNCTION(Runtime_ArrayConstructorWithSubclassing) { - HandleScope scope(isolate); - DCHECK(args.length() >= 2); - int args_length = args.length(); - 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::null(), &caller_args); + return ArrayConstructorCommon(isolate, constructor, site, caller_args); } @@ -1188,7 +1161,7 @@ RUNTIME_FUNCTION(Runtime_InternalArrayConstructor) { DCHECK(arg_count == caller_args->length()); } #endif - return ArrayConstructorCommon(isolate, constructor, constructor, + return ArrayConstructorCommon(isolate, constructor, Handle::null(), caller_args); } diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index f173cc96feb..57d450d22e1 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -465,7 +465,6 @@ namespace internal { \ /* Arrays */ \ F(ArrayConstructor, -1, 1) \ - F(ArrayConstructorWithSubclassing, -1, 1) \ F(InternalArrayConstructor, -1, 1) \ \ /* Literals */ \ diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc index dd79a4e1a2d..cc927145226 100644 --- a/src/x64/code-stubs-x64.cc +++ b/src/x64/code-stubs-x64.cc @@ -4636,30 +4636,8 @@ void ArrayConstructorStub::Generate(MacroAssembler* masm) { __ bind(&no_info); GenerateDispatchToArrayStub(masm, DISABLE_ALLOCATION_SITES); - // Subclassing __ bind(&subclassing); - __ 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); + __ TailCallRuntime(Runtime::kThrowArrayNotSubclassableError, 0, 1); } diff --git a/test/mjsunit/harmony/classes-subclass-arrays.js b/test/mjsunit/harmony/classes-subclass-arrays.js deleted file mode 100644 index e0363c715bc..00000000000 --- a/test/mjsunit/harmony/classes-subclass-arrays.js +++ /dev/null @@ -1,150 +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'; - -(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 new file mode 100644 index 00000000000..2436a2d0c38 --- /dev/null +++ b/test/mjsunit/harmony/regress/regress-3930.js @@ -0,0 +1,28 @@ +// 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);