Fix Function subclassing.

Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype, etc.).
Now when an instance of a Function subclass is created it gets initial map that corresponds
to the language mode of the function body. The language mode dependent maps are cached as
special transitions on initial map of the subclass constructor.

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

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

[Igor Sheludko, 2015-12-08 15:09:48]

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

[Igor Sheludko, 2015-12-08 15:09:49]

PTAL

[Igor Sheludko, 2015-12-08 15:12:56]

Description was changed from

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

BUG=v8:4597
LOG=Y
==========

to

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

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

[Toon Verwaest, 2015-12-08 20:01:08]

looking good.

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

https://codereview.chromium.org/1510753005/diff/1/src/objects.cc#newcode9203
src/objects.cc:9203: function_map = isolate->sloppy_function_map();
What about making the initial map be the sloppy map? Isn't that already the
case? If we do that, we probably don't need to transition in most cases.

https://codereview.chromium.org/1510753005/diff/1/src/objects.cc#newcode9229
src/objects.cc:9229: map->set_owns_descriptors(false);
I don't immediately understand this ...
If we share the descriptor array, the initial map should lose ownership, not the
target map.

https://codereview.chromium.org/1510753005/diff/1/src/objects.cc#newcode9232
src/objects.cc:9232: map->set_prototype(initial_map->prototype());
Is this correct for strong-mode?...

[Igor Sheludko, 2015-12-09 17:44:03]

Patchset #2 (id:20001) has been deleted

[Toon Verwaest, 2015-12-09 21:40:03]

Looking good. I'm pretty sure it works, but could you please add a test to
test/mjsunit/harmony/reflect-construct.js to test
well_known_intrinsic_constructor with strict/strong-mode functions? E.g., add
["Function", ['"use strict"']] to the list below "Function".

https://codereview.chromium.org/1510753005/diff/40001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1510753005/diff/40001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1070:
sloppy_function_map_writable_prototype_->set_constructor_or_backpointer(
SetConstructor?

https://codereview.chromium.org/1510753005/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1510753005/diff/40001/src/objects.cc#newcode8932
src/objects.cc:8932: DCHECK_EQ(*map,
JSFunction::cast(constructor)->initial_map());
so initial_map == map || map == strict_function_map || ...?

[Igor Sheludko, 2015-12-10 10:49:01]

Patchset #4 (id:80001) has been deleted

[Igor Sheludko, 2015-12-10 10:50:06]

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

[commit-bot: I haz the power, 2015-12-10 10:50:17]

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

[Igor Sheludko, 2015-12-10 10:50:41]

PTAL again.

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

https://codereview.chromium.org/1510753005/diff/1/src/objects.cc#newcode9203
src/objects.cc:9203: function_map = isolate->sloppy_function_map();
On 2015/12/08 20:01:08, Toon Verwaest wrote:
> What about making the initial map be the sloppy map? Isn't that already the
> case? If we do that, we probably don't need to transition in most cases.

Done.

https://codereview.chromium.org/1510753005/diff/1/src/objects.cc#newcode9229
src/objects.cc:9229: map->set_owns_descriptors(false);
On 2015/12/08 20:01:08, Toon Verwaest wrote:
> I don't immediately understand this ...
> If we share the descriptor array, the initial map should lose ownership, not
the
> target map.

Done.

https://codereview.chromium.org/1510753005/diff/40001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1510753005/diff/40001/src/bootstrapper.cc#new...
src/bootstrapper.cc:1070:
sloppy_function_map_writable_prototype_->set_constructor_or_backpointer(
On 2015/12/09 21:40:03, Toon Verwaest wrote:
> SetConstructor?

Done.

https://codereview.chromium.org/1510753005/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1510753005/diff/40001/src/objects.cc#newcode8932
src/objects.cc:8932: DCHECK_EQ(*map,
JSFunction::cast(constructor)->initial_map());
On 2015/12/09 21:40:03, Toon Verwaest wrote:
> so initial_map == map || map == strict_function_map || ...?

Done.

[commit-bot: I haz the power, 2015-12-10 11:16:43]

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

[commit-bot: I haz the power, 2015-12-10 11:16:44]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2015-12-10 13:10:54]

lgtm

https://codereview.chromium.org/1510753005/diff/100001/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1510753005/diff/100001/src/bootstrapper.cc#ne...
src/bootstrapper.cc:1097:
native_context()->strong_function_map()->SetConstructor(*function_function);
I think we can drop the special function-case in EmitClassOf now ...

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc#newcode...
src/objects.cc:9289: default:
case SLOPPY:

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc#newcode...
src/objects.cc:9307: 
if (TransitionArray::CanHaveMoreTransitions(initial_map)) {
...
}

https://codereview.chromium.org/1510753005/diff/100001/test/mjsunit/es6/class...
File test/mjsunit/es6/classes-subclass-builtins.js (right):

https://codereview.chromium.org/1510753005/diff/100001/test/mjsunit/es6/class...
test/mjsunit/es6/classes-subclass-builtins.js:82: if (args[args.length -
1].indexOf("use strong") >= 0) return;
if(strong) return assertThrows(()=>{ this.a = 10 }, TypeError)

[Igor Sheludko, 2015-12-10 16:25:04]

The CQ bit was checked by ishell@chromium.org

[Igor Sheludko, 2015-12-10 16:25:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
verwaest@chromium.org
Link to the patchset: https://codereview.chromium.org/1510753005/#ps120001
(title: "Adressing comments")

[commit-bot: I haz the power, 2015-12-10 16:25:15]

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

[Igor Sheludko, 2015-12-10 16:39:47]

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc#newcode...
src/objects.cc:9289: default:
On 2015/12/10 13:10:54, Toon Verwaest wrote:
> case SLOPPY:

Does not work: this forces me to add "case STRONG_BIT". I added
STATIC_ASSERT(LANGUAGE_END == 3) instead.

https://codereview.chromium.org/1510753005/diff/100001/src/objects.cc#newcode...
src/objects.cc:9307: 
On 2015/12/10 13:10:54, Toon Verwaest wrote:
> if (TransitionArray::CanHaveMoreTransitions(initial_map)) {
> ...
> }

Done.

https://codereview.chromium.org/1510753005/diff/100001/test/mjsunit/es6/class...
File test/mjsunit/es6/classes-subclass-builtins.js (right):

https://codereview.chromium.org/1510753005/diff/100001/test/mjsunit/es6/class...
test/mjsunit/es6/classes-subclass-builtins.js:82: if (args[args.length -
1].indexOf("use strong") >= 0) return;
On 2015/12/10 13:10:54, Toon Verwaest wrote:
> if(strong) return assertThrows(()=>{ this.a = 10 }, TypeError)

Done.

[commit-bot: I haz the power, 2015-12-10 17:08:17]

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

[commit-bot: I haz the power, 2015-12-10 17:08:18]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/8812)

[ulan, 2015-12-10 17:21:08]

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

[ulan, 2015-12-10 17:21:09]

heap lgtm

[Igor Sheludko, 2015-12-10 17:26:05]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2015-12-10 17:26:14]

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

[commit-bot: I haz the power, 2015-12-10 17:27:48]

Description was changed from

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

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

to

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

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

[commit-bot: I haz the power, 2015-12-10 17:27:49]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2015-12-10 17:28:13]

Description was changed from

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

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

to

==========
Fix Function subclassing.

Function subclasses did not have function properties installed (name, prototype,
etc.).
Now when an instance of a Function subclass is created it gets initial map that
corresponds
to the language mode of the function body. The language mode dependent maps are
cached as
special transitions on initial map of the subclass constructor.

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

Committed: https://crrev.com/dddcd0ac17be9e24df15fc6ae4993f2b75bc256d
Cr-Commit-Position: refs/heads/master@{#32764}
==========

[commit-bot: I haz the power, 2015-12-10 17:28:14]

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