[strong] Object literals create strong objects

[strong] Object literals create strong objects

R=dslomov@chromium.org
BUG=v8:3956
LOG=N

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

[arv (Not doing code reviews), 2015-05-13 16:39:37]

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

[arv (Not doing code reviews), 2015-05-13 16:39:38]

Does this add any restrictions or is it just about using the Map with the strong
bit?

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
File test/mjsunit/strong/literals.js (right):

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
test/mjsunit/strong/literals.js:20: assertTrue(%IsStrong({a: 0, b: 0}));
Please add tests with accessors, methods, __proto__, super and computed property
names.

[Dmitry Lomov (no reviews), 2015-05-18 09:15:57]

Overall looks good but I have one question

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1015: CacheInitialJSArrayMaps(native_context(), initial_map,
false);
Where do you call this function with 'true'?

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
File test/mjsunit/strong/literals.js (right):

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
test/mjsunit/strong/literals.js:20: assertTrue(%IsStrong({a: 0, b: 0}));
On 2015/05/13 16:39:37, arv wrote:
> Please add tests with accessors, methods, __proto__, super and computed
property
> names.

Also regexps, if I understand the patch correctly.

[rossberg, 2015-05-18 09:32:37]

On 2015/05/13 16:39:38, arv wrote:
> Does this add any restrictions or is it just about using the Map with the
strong
> bit?

It's only about initialising the bit.

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1015: CacheInitialJSArrayMaps(native_context(), initial_map,
false);
On 2015/05/18 09:15:56, Dmitry Lomov (chromium) wrote:
> Where do you call this function with 'true'?

In JSFunction::SetInstancePrototype (objects.cc:10365), where it also calls it
with false again. To be honest, I don't understand this -- why is the
initialisation of the (weak) array maps happening in two places?

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
File test/mjsunit/strong/literals.js (right):

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
test/mjsunit/strong/literals.js:20: assertTrue(%IsStrong({a: 0, b: 0}));
On 2015/05/13 16:39:37, arv wrote:
> Please add tests with accessors, methods, __proto__, super and computed
property
> names.

Done. Turns out that super does not work currently, because the object is
non-extensible when HomeObject is being set.

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
test/mjsunit/strong/literals.js:20: assertTrue(%IsStrong({a: 0, b: 0}));
On 2015/05/18 09:15:56, Dmitry Lomov (chromium) wrote:
> On 2015/05/13 16:39:37, arv wrote:
> > Please add tests with accessors, methods, __proto__, super and computed
> property
> > names.
> 
> Also regexps, if I understand the patch correctly.

Those don't work yet, see the existing tests below. (Or did you mean something
else?)

[Dmitry Lomov (no reviews), 2015-05-18 10:56:30]

lgtm

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1015: CacheInitialJSArrayMaps(native_context(), initial_map,
false);
On 2015/05/18 09:32:37, rossberg wrote:
> On 2015/05/18 09:15:56, Dmitry Lomov (chromium) wrote:
> > Where do you call this function with 'true'?
> 
> In JSFunction::SetInstancePrototype (objects.cc:10365), where it also calls it
> with false again. To be honest, I don't understand this -- why is the
> initialisation of the (weak) array maps happening in two places?

Sounds like a bug/oversight - ask Yang or Ulan?

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
File test/mjsunit/strong/literals.js (right):

https://codereview.chromium.org/1134333005/diff/1/test/mjsunit/strong/literal...
test/mjsunit/strong/literals.js:20: assertTrue(%IsStrong({a: 0, b: 0}));
On 2015/05/18 09:32:37, rossberg wrote:
> On 2015/05/18 09:15:56, Dmitry Lomov (chromium) wrote:
> > On 2015/05/13 16:39:37, arv wrote:
> > > Please add tests with accessors, methods, __proto__, super and computed
> > property
> > > names.
> > 
> > Also regexps, if I understand the patch correctly.
> 
> Those don't work yet, see the existing tests below. (Or did you mean something
> else?)

I see, missed that.

[rossberg, 2015-05-18 12:01:26]

PTAL

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):

https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode...
src/bootstrapper.cc:1015: CacheInitialJSArrayMaps(native_context(), initial_map,
false);
On 2015/05/18 10:56:30, Dmitry Lomov (chromium) wrote:
> On 2015/05/18 09:32:37, rossberg wrote:
> > On 2015/05/18 09:15:56, Dmitry Lomov (chromium) wrote:
> > > Where do you call this function with 'true'?
> > 
> > In JSFunction::SetInstancePrototype (objects.cc:10365), where it also calls
it
> > with false again. To be honest, I don't understand this -- why is the
> > initialisation of the (weak) array maps happening in two places?
> 
> Sounds like a bug/oversight - ask Yang or Ulan?

Toon pointed out that the code here is to capture assignments to
Array.prototype. Added the setup in bootstrapper as well, plus a test (yet
inactive).

[Dmitry Lomov (no reviews), 2015-05-18 12:35:18]

lgtm

[rossberg, 2015-05-18 12:57:36]

The CQ bit was checked by rossberg@chromium.org

[commit-bot: I haz the power, 2015-05-18 12:57:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1134333005/60001

[commit-bot: I haz the power, 2015-05-18 13:23:37]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-05-18 13:23:54]

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

[arv (Not doing code reviews), 2015-05-18 14:50:21]

LGTM

[Michael Achenbach, 2015-05-19 07:35:54]

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

The reason for reverting is: [Sheriff] Segmentation fault on arm64 with nosnap:
http://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20arm64%20-%20s....