Preparse lazy function parameters

Preparse lazy function parameters

Parameters of a lazily parsed function used to be parsed eagerly,
and parameter handling was split between
Parser::ParseFunctionLiteral and ParseEagerFunctionBody, leading
into inconsistencies.

After this CL, we preparse (lazy parse) the parameters of lazily
parsed functions.

(For arrow functions, we cannot do that ofc.)

This is needed for later features (PreParser with scope analysis).

BUG=

[marja, 2016-10-18 10:32:43]

Description was changed from

==========
Parse / PreParse function parameters and body consistently.

WIP- DO NOT COMMIT

Currently, parameters are parsed eagerly even if the function body is
parsed lazily, and parameter handling is split between
Parser::ParseFunctionLiteral and ParseEagerFunctionbody.

This is needed for later features (PreParser with scope analysis).

BUG=
==========

to

==========
Parse / PreParse function parameters and body consistently.

Parameters of a lazily parsed function used to be parsed eagerly,
and parameter handling was split between
Parser::ParseFunctionLiteral and ParseEagerFunctionBody, leading
into inconsistencies.

After this CL, we preparse (lazy parse) the parameters of lazily
parsed functions.

(For arrow functions, we cannot do that ofc.)

This is needed for later features (PreParser with scope analysis).

BUG=
==========

[marja, 2016-10-18 11:26:46]

Description was changed from

==========
Parse / PreParse function parameters and body consistently.

Parameters of a lazily parsed function used to be parsed eagerly,
and parameter handling was split between
Parser::ParseFunctionLiteral and ParseEagerFunctionBody, leading
into inconsistencies.

After this CL, we preparse (lazy parse) the parameters of lazily
parsed functions.

(For arrow functions, we cannot do that ofc.)

This is needed for later features (PreParser with scope analysis).

BUG=
==========

to

==========
Preparse lazy function parameters

Parameters of a lazily parsed function used to be parsed eagerly,
and parameter handling was split between
Parser::ParseFunctionLiteral and ParseEagerFunctionBody, leading
into inconsistencies.

After this CL, we preparse (lazy parse) the parameters of lazily
parsed functions.

(For arrow functions, we cannot do that ofc.)

This is needed for later features (PreParser with scope analysis).

BUG=
==========

[marja, 2016-10-18 11:33:24]

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

[commit-bot: I haz the power, 2016-10-18 11:33:30]

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

[marja, 2016-10-18 11:33:54]

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

[commit-bot: I haz the power, 2016-10-18 11:34:59]

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

[commit-bot: I haz the power, 2016-10-18 11:35:00]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/14782)
  v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...)
  v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...)
  v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
  v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...)
  v8_presubmit on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/26731)
  v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)

[marja, 2016-10-18 11:43:50]

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

[commit-bot: I haz the power, 2016-10-18 11:44:01]

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

[marja, 2016-10-18 11:58:46]

ptal

[commit-bot: I haz the power, 2016-10-18 12:16:35]

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

[commit-bot: I haz the power, 2016-10-18 12:16:36]

Dry run: This issue passed the CQ dry run.

[Toon Verwaest, 2016-10-18 15:24:28]

Overall this looks good. I'm not sure it's easier to understand to have a single
SkipLazyFunction than having helpers for checking the lazy function cache and
storing things in the cache, with a SkipLazyArrowFunction and
SkipLazyFunctionLiteral. Also, what about dropping Lazy from the name? :)

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc
File src/parsing/parser.cc (left):

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc#...
src/parsing/parser.cc:2720: // function, since the function can declare itself
strict.
I guess part of this comment still makes sense?

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc
File src/parsing/parser.cc (right):

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc#...
src/parsing/parser.cc:2672: body = ParseFunctionParametersAndBodyEagerly(
Why not just ParseFunction to match SkipLazyFunction?

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc#...
src/parsing/parser.cc:2760:
function_scope->set_start_position(entry.start_pos());
Why not just skip the LPAREN in the outside already to make the start-position
known; so we don't need to cache it?

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc#...
src/parsing/parser.cc:2764: function_scope->end_position() -
function_scope->start_position();
that's not exactly correct right?

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/parser.cc#...
src/parsing/parser.cc:2799: DCHECK_NE(kNoSourcePosition,
function_scope->start_position());
I'd move this to the start of the function already

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/preparser.cc
File src/parsing/preparser.cc (right):

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/preparser....
src/parsing/preparser.cc:113: if ((kind & FunctionKind::kArrowFunction) == 0) {
IsArrowFunction makes it easier to understand

https://codereview.chromium.org/2411793003/diff/300001/src/parsing/preparser....
src/parsing/preparser.cc:145: if ((kind & FunctionKind::kArrowFunction) == 0) {
same here

[marja, 2016-11-16 10:44:38]

Closing this, since https://codereview.chromium.org/2472063002 from
verwaest@ contains the relevant changes from this cl.