Don't crash when preparsing destructured arguments

Don't crash when preparsing destructured arguments

This adds the materialized literal count accumulated while parsing the
parameters (in the parser proper) to that accumulated by the preparser.

This should have been caught in cctest/test-parsing, but it's not covered
because the parsing tests call directly into the preparser rather than
using Parser::ParseFunctionLiteral (which fully-parses the parameters
and then calls into the preparser to skip over the function body).

Note that this further-inflates the materialized literal count for
functions with destructured arguments, since some of the counted
literals are actually binding patterns. But that's not specific to
binding patterns in formal parameters: it happens in function bodies, too.

BUG=v8:4400, v8:4407
LOG=n

Committed: https://crrev.com/7485da7ace55cf4318f0a3a02a54fed04bed2a7a
Cr-Commit-Position: refs/heads/master@{#30868}

[adamk, 2015-09-18 21:24:31]

adamk@chromium.org changed reviewers:
+ caitpotter88@gmail.com, rossberg@chromium.org

[caitp (gmail), 2015-09-18 21:30:09]

Looks okay to me

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc#newcode4203
src/parser.cc:4203: if (function_state.materialized_literal_count() > 0) {
FunctionState's materialized_literal_count() never returns less than 0, so it
should be okay to get rid of the if block.

[caitp (gmail), 2015-09-18 21:38:17]

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc#newcode4203
src/parser.cc:4203: if (function_state.materialized_literal_count() > 0) {
On 2015/09/18 21:30:08, caitp wrote:
> FunctionState's materialized_literal_count() never returns less than 0, so it
> should be okay to get rid of the if block.

In fact, this seems to be true for FormalParameters too, so maybe both the
conditional checks should be taken out?

[adamk, 2015-09-18 23:23:03]

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/1350913005/diff/1/src/parser.cc#newcode4203
src/parser.cc:4203: if (function_state.materialized_literal_count() > 0) {
On 2015/09/18 21:38:16, caitp wrote:
> On 2015/09/18 21:30:08, caitp wrote:
> > FunctionState's materialized_literal_count() never returns less than 0, so
it
> > should be okay to get rid of the if block.
> 
> In fact, this seems to be true for FormalParameters too, so maybe both the
> conditional checks should be taken out?

Sounds good, combined into a single statement.

[rossberg, 2015-09-22 15:28:15]

lgtm

[adamk, 2015-09-22 17:19:05]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2015-09-22 17:19:11]

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

[commit-bot: I haz the power, 2015-09-22 17:43:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-22 17:43:54]

Patchset 2 (id:??) landed as
https://crrev.com/7485da7ace55cf4318f0a3a02a54fed04bed2a7a
Cr-Commit-Position: refs/heads/master@{#30868}