[es6] Better support for built-ins subclassing.

[es6] Better support for built-ins subclassing.

Create proper initial map for original constructor (new.target) instead of doing prototype transition on the base constructor's initial map. This approach fixes in-object slack tracking for subclass instances.
This CL also fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y

Committed: https://crrev.com/cd5f48302a502154a0106d12e3066bd563c6340c
Cr-Commit-Position: refs/heads/master@{#31680}

[Igor Sheludko, 2015-10-26 14:14:12]

ishell@chromium.org changed reviewers:
+ verwaest@chromium.org

[Igor Sheludko, 2015-10-26 14:14:13]

PTAL, other ports are on the way.

[Toon Verwaest, 2015-10-26 14:16:44]

Description was changed from

==========
[es6] Support for built-ins subclassing.

BUG=v8:3101
LOG=Y
==========

to

==========
[es6] Support for built-ins subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

[Igor Sheludko, 2015-10-26 14:31:25]

Description was changed from

==========
[es6] Support for built-ins subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

to

==========
[es6] Support for String built-in subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

[Igor Sheludko, 2015-10-26 14:31:54]

Description was changed from

==========
[es6] Support for String built-in subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

to

==========
[es6] Support for built-in subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

[Toon Verwaest, 2015-10-26 15:10:57]

Looking good. Some minor comments.

https://codereview.chromium.org/1427483002/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/objects.cc#newcode11842
src/objects.cc:11842:
original_constructor->shared()->CalculateInObjectProperties(
Shouldn't we add up the expected number of properties (expected_nof_properties)
from the entire class hierarchy? I guess we need a
"CalculateInstanceSizeForDerivedClass".

https://codereview.chromium.org/1427483002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:1049: return isolate->global_proxy();
The comment above seems out-of-date. We can probably just return
isolate->heap()->undefined_value() here, and add a comment that the Function
constructor creates its own functions by compiling.

To support subclassing Function we probably still will want to execute the code
below to create an initial map on the origin_constructor; and use it to swap the
map of the resulting function after compilation in tune FunctionConstructor.

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1524: __ Push(rdi);  // argument 2/1: constructor
function
I don't understand 2/1, 3/2...

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1527: __ CallRuntime(Runtime::kNewObject, 2);
Please add some tests here that subclassing now works.

[Igor Sheludko, 2015-10-27 12:00:24]

Patchset #2 (id:20001) has been deleted

[Igor Sheludko, 2015-10-27 12:01:17]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 12:01:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/40001

[commit-bot: I haz the power, 2015-10-27 12:02:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 12:02:26]

Dry run: Try jobs failed on following builders:
  v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...)
  v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mipsel_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_mac_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/11176)

[ulan, 2015-10-27 12:03:11]

ulan@chromium.org changed reviewers:
+ ulan@chromium.org

[ulan, 2015-10-27 12:03:11]

Heap part lgtm.

[Igor Sheludko, 2015-10-27 12:53:49]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 12:53:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/80001

[Igor Sheludko, 2015-10-27 12:55:16]

Description was changed from

==========
[es6] Support for built-in subclassing.

BUG=v8:3101, v8:3330
LOG=Y
==========

to

==========
[es6] Support for built-in subclassing.

This CL fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y
==========

[Igor Sheludko, 2015-10-27 12:56:12]

Adressed comments and added ports. The CL changed significantly.

https://codereview.chromium.org/1427483002/diff/1/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/objects.cc#newcode11842
src/objects.cc:11842:
original_constructor->shared()->CalculateInObjectProperties(
On 2015/10/26 15:10:57, Toon Verwaest wrote:
> Shouldn't we add up the expected number of properties
(expected_nof_properties)
> from the entire class hierarchy? I guess we need a
> "CalculateInstanceSizeForDerivedClass".

Done.

https://codereview.chromium.org/1427483002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:1049: return isolate->global_proxy();
On 2015/10/26 15:10:57, Toon Verwaest wrote:
> The comment above seems out-of-date. We can probably just return
> isolate->heap()->undefined_value() here, and add a comment that the Function
> constructor creates its own functions by compiling.
> 
> To support subclassing Function we probably still will want to execute the
code
> below to create an initial map on the origin_constructor; and use it to swap
the
> map of the resulting function after compilation in tune FunctionConstructor.

I'll address function subclassing in a separate CL.

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1524: __ Push(rdi);  // argument 2/1: constructor
function
On 2015/10/26 15:10:57, Toon Verwaest wrote:
> I don't understand 2/1, 3/2...

Done.

https://codereview.chromium.org/1427483002/diff/1/src/x64/builtins-x64.cc#new...
src/x64/builtins-x64.cc:1527: __ CallRuntime(Runtime::kNewObject, 2);
On 2015/10/26 15:10:57, Toon Verwaest wrote:
> Please add some tests here that subclassing now works.

Done.

[commit-bot: I haz the power, 2015-10-27 12:58:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 12:58:18]

Dry run: Try jobs failed on following builders:
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/9458)

[Igor Sheludko, 2015-10-27 13:32:23]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 13:32:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/100001

[commit-bot: I haz the power, 2015-10-27 13:44:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 13:44:17]

Dry run: Try jobs failed on following builders:
  v8_linux_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/11052)

[Igor Sheludko, 2015-10-27 13:45:28]

Patchset #5 (id:100001) has been deleted

[Igor Sheludko, 2015-10-27 13:45:51]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-27 13:45:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/120001

[commit-bot: I haz the power, 2015-10-27 14:04:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-27 14:04:54]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2015-10-28 21:14:14]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-28 21:14:18]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/140001

[commit-bot: I haz the power, 2015-10-28 21:34:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-28 21:34:40]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2015-10-29 12:54:46]

lgtm, but the CL description is a bit off :)

https://codereview.chromium.org/1427483002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1427483002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:995: if (function->has_initial_map() &&
original_function->has_initial_map()) {
Do we still need this additional special case now it's already handled below?

[Igor Sheludko, 2015-10-29 15:26:06]

Description was changed from

==========
[es6] Support for built-in subclassing.

This CL fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y
==========

to

==========
[es6] Better support for built-in subclassing.

Create proper initial map for original constructor (new.target) instead of doing
prototype transition on the base constructor's initial map. This approach fixes
in-object slack tracking for subclass instances.
This CL also fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y
==========

[Igor Sheludko, 2015-10-29 15:26:22]

https://codereview.chromium.org/1427483002/diff/140001/src/runtime/runtime-ob...
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/1427483002/diff/140001/src/runtime/runtime-ob...
src/runtime/runtime-object.cc:995: if (function->has_initial_map() &&
original_function->has_initial_map()) {
On 2015/10/29 12:54:46, Toon Verwaest wrote:
> Do we still need this additional special case now it's already handled below?

Done.

[Igor Sheludko, 2015-10-29 15:26:28]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 15:26:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/160001

[Igor Sheludko, 2015-10-29 15:31:01]

Description was changed from

==========
[es6] Better support for built-in subclassing.

Create proper initial map for original constructor (new.target) instead of doing
prototype transition on the base constructor's initial map. This approach fixes
in-object slack tracking for subclass instances.
This CL also fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y
==========

to

==========
[es6] Better support for built-ins subclassing.

Create proper initial map for original constructor (new.target) instead of doing
prototype transition on the base constructor's initial map. This approach fixes
in-object slack tracking for subclass instances.
This CL also fixes subclassing from String.

BUG=v8:3101, v8:3330
LOG=Y
==========

[commit-bot: I haz the power, 2015-10-29 15:52:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 15:52:35]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2015-10-29 19:46:10]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 19:46:27]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/180001

[commit-bot: I haz the power, 2015-10-29 20:14:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 20:14:57]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2015-10-29 22:20:07]

Patchset #8 (id:180001) has been deleted

[Igor Sheludko, 2015-10-29 22:20:38]

The CQ bit was checked by ishell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-29 22:20:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/200001

[commit-bot: I haz the power, 2015-10-29 22:52:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-29 22:52:35]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2015-10-30 10:56:44]

The CQ bit was checked by ishell@chromium.org

[Igor Sheludko, 2015-10-30 10:56:47]

The patchset sent to the CQ was uploaded after l-g-t-m from ulan@chromium.org,
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1427483002/#ps200001
(title: "Test cleanup")

[commit-bot: I haz the power, 2015-10-30 10:56:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1427483002/200001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1427483002/200001

[commit-bot: I haz the power, 2015-10-30 10:58:08]

Committed patchset #8 (id:200001)

[commit-bot: I haz the power, 2015-10-30 10:58:40]

Patchset 8 (id:??) landed as
https://crrev.com/cd5f48302a502154a0106d12e3066bd563c6340c
Cr-Commit-Position: refs/heads/master@{#31680}

[Benedikt Meurer, 2015-10-30 11:35:55]

A revert of this CL (patchset #8 id:200001) has been created in
https://codereview.chromium.org/1416943005/ by bmeurer@chromium.org.

The reason for reverting is: Breaks test on win32
nosnap.http://build.chromium.org/p/client.v8/builders/V8%20Win32%20-%20nosnap%20-%20shared/builds/9680.

[Michael Achenbach, 2015-10-30 11:44:25]

A revert of this CL (patchset #8 id:200001) has been created in
https://codereview.chromium.org/1417423005/ by machenbach@chromium.org.

The reason for reverting is: [Sheriff] nosnap failures. E.g.:
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20nosnap/builds....