[ast] Count index keys in AST not runtime.

[ast] Count index keys in AST not runtime.

We do not want to reserve space in the backing store for index keys.
Count index keys during creation of the BoilerplateDescription, and
substract them for the backing store size.

Correctly count index keys after encountering a property with
a computed name during object literal creation.

R=verwaest@chromium.org

BUG=v8:5625
Review-Url: https://codereview.chromium.org/2651523002
Cr-Commit-Position: refs/heads/master@{#42598}
Committed: https://chromium.googlesource.com/v8/v8/+/0d1e0a15214505f017da068cdcd9ca38c9cf53e9

[Franzi, 2017-01-22 20:58:06]

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

[commit-bot: I haz the power, 2017-01-22 20:58:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Franzi, 2017-01-22 21:26:57]

Description was changed from

==========
[ast] Count index keys in ast not runtime.

We do not want to reserve space in the backing store for index keys.
Count index keys during creation of the BoilerplateDescription, and
substract them for the backing store size.

Correctly count index keys fter seeing a property with
a computed name during object literal creation.

R=verwaest@chromium.org

BUG=v8:5625
==========

to

==========
[ast] Count index keys in AST not runtime.

We do not want to reserve space in the backing store for index keys.
Count index keys during creation of the BoilerplateDescription, and
substract them for the backing store size.

Correctly count index keys after encountering a property with
a computed name during object literal creation.

R=verwaest@chromium.org

BUG=v8:5625
==========

[commit-bot: I haz the power, 2017-01-22 21:27:59]

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

[commit-bot: I haz the power, 2017-01-22 21:28:00]

Dry run: This issue passed the CQ dry run.

[Franzi, 2017-01-22 21:32:13]

Hi Toon, 

I removed the loop that counts index keys in the runtime function and do that
instead during parsing. 

PTAL. 

Thanks, 

Franzi

[Toon Verwaest, 2017-01-22 21:39:16]

lgtm with comment

https://codereview.chromium.org/2651523002/diff/1/src/ast/ast.cc
File src/ast/ast.cc (right):

https://codereview.chromium.org/2651523002/diff/1/src/ast/ast.cc#newcode596
src/ast/ast.cc:596: if (key->ToArrayIndex(&element_index)) {
I guess you need AsArrayIndex here to also catch string-literals convertible to
array indices, like '3':1. Extend the test to cover this case.

[Franzi, 2017-01-22 22:52:10]

On 2017/01/22 21:39:16, Toon Verwaest wrote:
> lgtm with comment
> 
> https://codereview.chromium.org/2651523002/diff/1/src/ast/ast.cc
> File src/ast/ast.cc (right):
> 
> https://codereview.chromium.org/2651523002/diff/1/src/ast/ast.cc#newcode596
> src/ast/ast.cc:596: if (key->ToArrayIndex(&element_index)) {
> I guess you need AsArrayIndex here to also catch string-literals convertible
to
> array indices, like '3':1. Extend the test to cover this case.

I have a test for '2'. 

property->key()->AsLiteral()->value()
turns the key into a smi, even it it comes in as a string. Am I missing
something?

[Toon Verwaest, 2017-01-23 03:49:15]

'2' isn't actually dealt with by this loop, but by the loop below that walks
over properties again. That one does AsArrayIndex. You should add a '4' after 3
which I presume will be wrongly counted as inobject property.

[Toon Verwaest, 2017-01-23 03:57:01]

Mh, perhaps that AsArrayIndex is just outdated? In that case add a
DCHECK(!...AsArrayIndex)); in case ToArrayIndex is false, and remove the
AsArrayIndex below.

[Franzi, 2017-01-23 09:16:28]

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

[commit-bot: I haz the power, 2017-01-23 09:16:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Franzi, 2017-01-23 09:24:44]

On 2017/01/23 03:57:01, Toon Verwaest wrote:
> Mh, perhaps that AsArrayIndex is just outdated? In that case add a
> DCHECK(!...AsArrayIndex)); in case ToArrayIndex is false, and remove the
> AsArrayIndex below.

Oh, I found it. AsArrayIndex is needed for getters and setters. Added a test and
included it in the loop. 

var o = {get 12() {return 2;}}

[commit-bot: I haz the power, 2017-01-23 09:48:12]

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

[commit-bot: I haz the power, 2017-01-23 09:48:13]

Dry run: This issue passed the CQ dry run.

[Franzi, 2017-01-23 12:06:46]

The CQ bit was checked by franzih@chromium.org

[Franzi, 2017-01-23 12:06:46]

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/2651523002/#ps20001
(title: "Treat getters with numbers as their name correctly.")

[commit-bot: I haz the power, 2017-01-23 12:06:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-23 12:08:34]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1485173206080690,
"parent_rev": "14f470cafd909c7c5805edc0257048f495491f66", "commit_rev":
"0d1e0a15214505f017da068cdcd9ca38c9cf53e9"}

[commit-bot: I haz the power, 2017-01-23 12:08:37]

Description was changed from

==========
[ast] Count index keys in AST not runtime.

We do not want to reserve space in the backing store for index keys.
Count index keys during creation of the BoilerplateDescription, and
substract them for the backing store size.

Correctly count index keys after encountering a property with
a computed name during object literal creation.

R=verwaest@chromium.org

BUG=v8:5625
==========

to

==========
[ast] Count index keys in AST not runtime.

We do not want to reserve space in the backing store for index keys.
Count index keys during creation of the BoilerplateDescription, and
substract them for the backing store size.

Correctly count index keys after encountering a property with
a computed name during object literal creation.

R=verwaest@chromium.org

BUG=v8:5625

Review-Url: https://codereview.chromium.org/2651523002
Cr-Commit-Position: refs/heads/master@{#42598}
Committed:
https://chromium.googlesource.com/v8/v8/+/0d1e0a15214505f017da068cdcd9ca38c9c...
==========

[commit-bot: I haz the power, 2017-01-23 12:08:38]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/0d1e0a15214505f017da068cdcd9ca38c9c...