[parser] Throw error when there are too many excluded properties

[parser] Throw error when there are too many excluded properties

Check that number of properties < Code:kMaxArguments when object
destructuring with a rest property otherwise throw an error.

BUG=v8:5549
Review-Url: https://codereview.chromium.org/2650863002
Cr-Commit-Position: refs/heads/master@{#42613}
Committed: https://chromium.googlesource.com/v8/v8/+/d90e6e12e68c93230c18454eafa981e5566b2128

[gsathya, 2017-01-23 21:22:14]

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

[commit-bot: I haz the power, 2017-01-23 21:22:17]

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

[gsathya, 2017-01-23 21:23:13]

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

[commit-bot: I haz the power, 2017-01-23 21:23:17]

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

[gsathya, 2017-01-23 21:26:07]

gsathya@chromium.org changed reviewers:
+ adamk@chromium.org

[adamk, 2017-01-23 21:47:45]

https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base...
src/parsing/parser-base.h:2597:
this->classifier()->RecordPatternError(Scanner::Location(pos, position()),
Can you add a comment with a pointer to the code in pattern-rewriter that forces
this restriction?

https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
test/cctest/test-parsing.cc:6995: // The test is quite slow, so run it with a
reduced set of flags.
How slow did this end up being?

Depending on your answer to that, it would be nice to show that Object spread
allows more than kMaxArguments.

[commit-bot: I haz the power, 2017-01-23 21:57:22]

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

[commit-bot: I haz the power, 2017-01-23 21:57:23]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-23 22:22:27]

https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base.h
File src/parsing/parser-base.h (right):

https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base...
src/parsing/parser-base.h:2597:
this->classifier()->RecordPatternError(Scanner::Location(pos, position()),
On 2017/01/23 21:47:45, adamk wrote:
> Can you add a comment with a pointer to the code in pattern-rewriter that
forces
> this restriction?

Done.

https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
test/cctest/test-parsing.cc:6995: // The test is quite slow, so run it with a
reduced set of flags.
On 2017/01/23 21:47:45, adamk wrote:
> How slow did this end up being?

This doubled the run time of the test (13s to 25s). 

> Depending on your answer to that, it would be nice to show that Object spread
> allows more than kMaxArguments.

We don't pass all the properties in this case, so not sure what this will be
testing?

[gsathya, 2017-01-23 22:22:35]

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

[commit-bot: I haz the power, 2017-01-23 22:22:40]

Dry run: 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 22:56:17]

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

[commit-bot: I haz the power, 2017-01-23 22:56:17]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-23 23:49:06]

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

[commit-bot: I haz the power, 2017-01-23 23:49:15]

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

[gsathya, 2017-01-23 23:49:15]

On 2017/01/23 22:22:27, gsathya wrote:
>
https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base.h
> File src/parsing/parser-base.h (right):
> 
>
https://codereview.chromium.org/2650863002/diff/20001/src/parsing/parser-base...
> src/parsing/parser-base.h:2597:
> this->classifier()->RecordPatternError(Scanner::Location(pos, position()),
> On 2017/01/23 21:47:45, adamk wrote:
> > Can you add a comment with a pointer to the code in pattern-rewriter that
> forces
> > this restriction?
> 
> Done.
> 
>
https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
> File test/cctest/test-parsing.cc (right):
> 
>
https://codereview.chromium.org/2650863002/diff/20001/test/cctest/test-parsin...
> test/cctest/test-parsing.cc:6995: // The test is quite slow, so run it with a
> reduced set of flags.
> On 2017/01/23 21:47:45, adamk wrote:
> > How slow did this end up being?
> 
> This doubled the run time of the test (13s to 25s). 
> 
> > Depending on your answer to that, it would be nice to show that Object
spread
> > allows more than kMaxArguments.
> 
> We don't pass all the properties in this case, so not sure what this will be
> testing?

skipped test in debug

[adamk, 2017-01-23 23:57:53]

lgtm

[commit-bot: I haz the power, 2017-01-24 00:27:48]

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

[commit-bot: I haz the power, 2017-01-24 00:27:49]

Dry run: This issue passed the CQ dry run.

[gsathya, 2017-01-24 00:28:35]

The CQ bit was checked by gsathya@chromium.org

[commit-bot: I haz the power, 2017-01-24 00:28:39]

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-24 00:30:36]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485217715450510,
"parent_rev": "a5913c9a8ec0460c050d10d6d158afc7f23898ad", "commit_rev":
"d90e6e12e68c93230c18454eafa981e5566b2128"}

[commit-bot: I haz the power, 2017-01-24 00:30:40]

Description was changed from

==========
[parser] Throw error when there are too many excluded properties

Check that number of properties < Code:kMaxArguments when object
destructuring with a rest property otherwise throw an error.

BUG=v8:5549
==========

to

==========
[parser] Throw error when there are too many excluded properties

Check that number of properties < Code:kMaxArguments when object
destructuring with a rest property otherwise throw an error.

BUG=v8:5549

Review-Url: https://codereview.chromium.org/2650863002
Cr-Commit-Position: refs/heads/master@{#42613}
Committed:
https://chromium.googlesource.com/v8/v8/+/d90e6e12e68c93230c18454eafa981e5566...
==========

[commit-bot: I haz the power, 2017-01-24 00:30:41]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/d90e6e12e68c93230c18454eafa981e5566...