[parser] Only track parsing-mode (and possibly switch to the preparser) in the parser

[parser] Only track parsing-mode (and possibly switch to the preparser) in the parser

Otherwise we could in theory abort preparsing to the preparser and preparse again before aborting again... We shouldn't have this mess; so only set up mode_ in the parser in the first place.

BUG=

Committed: https://crrev.com/56b8afc1c363926348f5cca798ac41f28692e82e
Cr-Commit-Position: refs/heads/master@{#40809}

[Toon Verwaest, 2016-11-07 14:35:09]

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

[Toon Verwaest, 2016-11-07 14:35:10]

ptal

[Toon Verwaest, 2016-11-07 14:36:07]

Description was changed from

==========
[parser] Only tracking parsing-mode (and possibly switch to the preparser) in
the parser

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

to

==========
[parser] Only track parsing-mode (and possibly switch to the preparser) in the
parser

Otherwise we could in theory abort preparsing to the preparser and preparse
again before aborting again... We shouldn't have this mess; so only set up mode_
in the parser in the first place.

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

[Toon Verwaest, 2016-11-07 14:38:13]

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser-base.h
File src/parsing/parser-base.h (left):

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser-base.h#o...
src/parsing/parser-base.h:4412: if (scope()->is_eval_scope()) mode_ =
PARSE_EAGERLY;
There is no equivalent in the new version. We're trying to drop all such special
heuristics and always preparse instead. It's not because declarations don't leak
outside that you couldn't use strict eval to for example eval an entire program.
And programs don't necessarily use all code, which is why preparsing makes sense
in the first place.

[vogelheim, 2016-11-07 14:56:17]

lgtm

(only style nitpicks; please decide as you see fit)

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser-base.h
File src/parsing/parser-base.h (left):

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser-base.h#o...
src/parsing/parser-base.h:4412: if (scope()->is_eval_scope()) mode_ =
PARSE_EAGERLY;
On 2016/11/07 14:38:13, Toon Verwaest wrote:
> There is no equivalent in the new version. We're trying to drop all such
special
> heuristics and always preparse instead. It's not because declarations don't
leak
> outside that you couldn't use strict eval to for example eval an entire
program.
> And programs don't necessarily use all code, which is why preparsing makes
sense
> in the first place.

Thanks for the comment; I wouldn't have understood this without... :)

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser.h
File src/parsing/parser.h (right):

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser.h#newcod...
src/parsing/parser.h:241: : parser_(parser), old_mode_(parser->mode()) {
Why not just access parser_->mode_, since we're directly accessing/modifying
parser_mode_ elsewhere in this class anyhow?

https://codereview.chromium.org/2479213002/diff/1/src/parsing/parser.h#newcod...
src/parsing/parser.h:1137: Mode mode_;
If Parser/parser.h "owns" the parsing mode now, it probably makes sense to move
the Mode enum from ParserBase<> into Parser, too.

https://codereview.chromium.org/2479213002/diff/1/src/parsing/preparser.h
File src/parsing/preparser.h (right):

https://codereview.chromium.org/2479213002/diff/1/src/parsing/preparser.h#new...
src/parsing/preparser.h:918: // Indicates that we won't switch from the
preparser to the preparser; we'll
Since this just duplicates the comments (because both functions serve a similar
purpose), maybe just write the comment + both declarations in one block?

[Toon Verwaest, 2016-11-07 16:09:41]

Addressed comments. I replaced Mode mode() with bool parse_lazily() so the
preparse/parser-base don't need to have a mode anymore.

[Toon Verwaest, 2016-11-07 16:09:44]

The CQ bit was checked by verwaest@chromium.org

[Toon Verwaest, 2016-11-07 16:09:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
vogelheim@chromium.org
Link to the patchset: https://codereview.chromium.org/2479213002/#ps20001
(title: "addressed comments")

[commit-bot: I haz the power, 2016-11-07 16:09:54]

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, 2016-11-07 16:34:51]

Description was changed from

==========
[parser] Only track parsing-mode (and possibly switch to the preparser) in the
parser

Otherwise we could in theory abort preparsing to the preparser and preparse
again before aborting again... We shouldn't have this mess; so only set up mode_
in the parser in the first place.

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

to

==========
[parser] Only track parsing-mode (and possibly switch to the preparser) in the
parser

Otherwise we could in theory abort preparsing to the preparser and preparse
again before aborting again... We shouldn't have this mess; so only set up mode_
in the parser in the first place.

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

[commit-bot: I haz the power, 2016-11-07 16:34:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-17 22:24:58]

Description was changed from

==========
[parser] Only track parsing-mode (and possibly switch to the preparser) in the
parser

Otherwise we could in theory abort preparsing to the preparser and preparse
again before aborting again... We shouldn't have this mess; so only set up mode_
in the parser in the first place.

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

to

==========
[parser] Only track parsing-mode (and possibly switch to the preparser) in the
parser

Otherwise we could in theory abort preparsing to the preparser and preparse
again before aborting again... We shouldn't have this mess; so only set up mode_
in the parser in the first place.

BUG=

Committed: https://crrev.com/56b8afc1c363926348f5cca798ac41f28692e82e
Cr-Commit-Position: refs/heads/master@{#40809}
==========

[commit-bot: I haz the power, 2016-11-17 22:24:59]

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