Refactor parser mode configuration for correctness

Refactor parser mode configuration for correctness

This patch refactors the parser and preparser interface to be more
readable and type-safe.  It has no behavior changes.

Previously, parsers and preparsers were configured via bitfield called
parser_flags in the Parser constructor, and flags in
PreParser::PreParseProgram, ParserApi::Parse, and ParserApi::PreParse.
This was error-prone in practice: six call sites passed incorrectly
typed values to this interface (a boolean FLAG value, a boolean false
and a boolean true value).  None of these errors were caught by the
compiler because it's just an "int".

The parser flags interface was also awkward because it encoded a
language mode, but the language mode was only used to turn on harmony
scoping or not -- it wasn't used to actually set the parser's language
mode.

Fundamentally these errors came in because of the desire for a
procedural parser interface, in ParserApi.  Because we need to be able
to configure the parser in various ways, the flags argument got added;
but no one understood how to use the flags properly.  Also they were
only used by constructors: callers packed bits, and the constructors
unpacked them into booleans on the parser or preparser.

The solution is to allow parser construction, configuration, and
invocation to be separated.  This patch does that.

It passes the existing tests.

BUG=

Committed: http://code.google.com/p/v8/source/detail?r=14151

[wingo, 2013-04-05 08:48:35]

I wrote you a novel in the commit log.  Enjoy!

[Michael Starzinger, 2013-04-05 10:30:33]

Looking good. Mostly nits.

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

https://codereview.chromium.org/13450007/diff/1/src/parser.cc#newcode5927
src/parser.cc:5927: return DoPreParse(source, &recorder);
There doesn't seem to be a particular need to have a separate DoPreParse helper
method here (especially since the splitting is not symmetrical to
ParseProgram/DoParseProgram), let's just inline DoPreParse into here.

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

https://codereview.chromium.org/13450007/diff/1/src/parser.h#newcode168
src/parser.h:168: // Pre-parser a character stream, and return full preparse
data.
nit: s/Pre-parser/Pre-parse/ (minus the 'r').

https://codereview.chromium.org/13450007/diff/1/src/preparser-api.cc
File src/preparser-api.cc (right):

https://codereview.chromium.org/13450007/diff/1/src/preparser-api.cc#newcode197
src/preparser-api.cc:197: preparser.PreParseProgram();
nit: Should fit into one line now.

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

https://codereview.chromium.org/13450007/diff/1/src/preparser.h#newcode679
src/preparser.h:679: bool allow_modules_;
The allow_modules_ flag seems to be unused and also is no longer initialized,
let's drop it.

https://codereview.chromium.org/13450007/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/13450007/diff/1/test/cctest/test-parsing.cc#n...
test/cctest/test-parsing.cc:404: preparser.PreParseProgram();
nit: Indentation is off.

[wingo, 2013-04-05 12:00:14]

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

https://codereview.chromium.org/13450007/diff/1/src/parser.cc#newcode5927
src/parser.cc:5927: return DoPreParse(source, &recorder);
On 2013/04/05 10:30:33, Michael Starzinger wrote:
> There doesn't seem to be a particular need to have a separate DoPreParse
helper
> method here (especially since the splitting is not symmetrical to
> ParseProgram/DoParseProgram), let's just inline DoPreParse into here.

Done.

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

https://codereview.chromium.org/13450007/diff/1/src/parser.h#newcode168
src/parser.h:168: // Pre-parser a character stream, and return full preparse
data.
On 2013/04/05 10:30:33, Michael Starzinger wrote:
> nit: s/Pre-parser/Pre-parse/ (minus the 'r').

Done.

https://codereview.chromium.org/13450007/diff/1/src/preparser-api.cc
File src/preparser-api.cc (right):

https://codereview.chromium.org/13450007/diff/1/src/preparser-api.cc#newcode197
src/preparser-api.cc:197: preparser.PreParseProgram();
On 2013/04/05 10:30:33, Michael Starzinger wrote:
> nit: Should fit into one line now.

Done.

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

https://codereview.chromium.org/13450007/diff/1/src/preparser.h#newcode679
src/preparser.h:679: bool allow_modules_;
On 2013/04/05 10:30:33, Michael Starzinger wrote:
> The allow_modules_ flag seems to be unused and also is no longer initialized,
> let's drop it.

Good catch!  Done.

https://codereview.chromium.org/13450007/diff/1/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):

https://codereview.chromium.org/13450007/diff/1/test/cctest/test-parsing.cc#n...
test/cctest/test-parsing.cc:404: preparser.PreParseProgram();
On 2013/04/05 10:30:33, Michael Starzinger wrote:
> nit: Indentation is off.

Done.

[Michael Starzinger, 2013-04-05 12:09:26]

LGTM. I'll land this.

[Michael Starzinger, 2013-04-05 13:01:23]

Committed patchset #2 manually as r14151 (presubmit successful).