Begin modernization of --harmony-modules

Begin modernization of --harmony-modules

The approach taken in this CL is to incrementally move toward the
currently-specced version of modules in ES6. The biggest change in this
patch is separating the parsing of modules from the parsing of scripts,
getting rid of the 'module' keyword and thus disallowing modules-in-scripts
as well as modules-in-modules.

The syntax supported by import/export declarations has not yet been significantly
changed, with the major exception being that import declarations require a string
as the 'from' part.

Most of the existing tests have been disabled, with a first new test added
in cctest/test-parsing.

BUG=v8:1569
LOG=n

Committed: https://crrev.com/aeb3a7174050e67267a4e2c123469f040400008e
Cr-Commit-Position: refs/heads/master@{#26299}

[adamk, 2015-01-26 23:03:16]

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

[adamk, 2015-01-26 23:03:17]

This is ready for at least a review of the approach taken. Will send a note to
v8-team-harmony about the approach as well.

[arv (Not doing code reviews), 2015-01-27 01:17:16]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2015-01-27 01:17:17]

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

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1340
src/parser.cc:1340: Module* module = ParseModuleUrl(CHECK_OK);
This is fine for now but this should not return a Module AST node.

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

https://codereview.chromium.org/881623002/diff/1/test/cctest/test-parsing.cc#...
test/cctest/test-parsing.cc:4722: CHECK(!parser.Parse());
Would it not be cleaner if we added ParseScript and ParseModule?

[adamk, 2015-01-27 01:58:37]

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

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1340
src/parser.cc:1340: Module* module = ParseModuleUrl(CHECK_OK);
On 2015/01/27 01:17:17, arv wrote:
> This is fine for now but this should not return a Module AST node.

Yeah, the API probably just needs to match ParseStatementList (filling up the
list of body statements). The set of AST nodes involved isn't quite clear to me
yet (which is why I've yet to delete any from ast.h).

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

https://codereview.chromium.org/881623002/diff/1/test/cctest/test-parsing.cc#...
test/cctest/test-parsing.cc:4722: CHECK(!parser.Parse());
On 2015/01/27 01:17:17, arv wrote:
> Would it not be cleaner if we added ParseScript and ParseModule?

This was extracted from a branch where I had the whole compilation pipeline
parameterized. The trouble is there's lots of code to be shared between the two
paths (even in the parser, as you see, it's mostly shared), so adding a bit to
CompilationInfo seems more readable than passing a bool "is_module" to a bunch
of helper methods. And definitely more clear than duplicating the code around
the edges.

Put another way: what code would ParseModule and ParseScript methods on the
parser contain?

[rossberg, 2015-01-27 20:00:56]

LGTM, only nits

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

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1273
src/parser.cc:1273: Module* Parser::ParseModuleVariable(bool* ok) {
Is this still used?

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1347
src/parser.cc:1347: // TODO(ES6): Add a let declaration for each name
Nit: misleading comment, as imports cannot be let-declarations; they are a new
form of declaration with a binding semantics completely different from anything
that exists in the language or implementation so far.

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode2311
src/parser.cc:2311: // Also detect attempts at 'let' declarations in sloppy
mode.
Nit: remove the "also"

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

https://codereview.chromium.org/881623002/diff/1/test/cctest/test-parsing.cc#...
test/cctest/test-parsing.cc:4722: CHECK(!parser.Parse());
On 2015/01/27 01:58:37, adamk wrote:
> On 2015/01/27 01:17:17, arv wrote:
> > Would it not be cleaner if we added ParseScript and ParseModule?
> 
> This was extracted from a branch where I had the whole compilation pipeline
> parameterized. The trouble is there's lots of code to be shared between the
two
> paths (even in the parser, as you see, it's mostly shared), so adding a bit to
> CompilationInfo seems more readable than passing a bool "is_module" to a bunch
> of helper methods. And definitely more clear than duplicating the code around
> the edges.
> 
> Put another way: what code would ParseModule and ParseScript methods on the
> parser contain?

I don't think we have a separate ParseEval either, so the current CL is at least
consistent.

[adamk, 2015-01-27 20:11:02]

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

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1273
src/parser.cc:1273: Module* Parser::ParseModuleVariable(bool* ok) {
On 2015/01/27 20:00:56, rossberg wrote:
> Is this still used?

No, it's not. I thought it might be useful for "import *" but it's simple enough
to re-add later.

I'll delete this for now, but note that I'm not yet deleting stuff from ast.h.

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode1347
src/parser.cc:1347: // TODO(ES6): Add a let declaration for each name
On 2015/01/27 20:00:55, rossberg wrote:
> Nit: misleading comment, as imports cannot be let-declarations; they are a new
> form of declaration with a binding semantics completely different from
anything
> that exists in the language or implementation so far.

Fair enough, s/let/appropriate/

https://codereview.chromium.org/881623002/diff/1/src/parser.cc#newcode2311
src/parser.cc:2311: // Also detect attempts at 'let' declarations in sloppy
mode.
On 2015/01/27 20:00:56, rossberg wrote:
> Nit: remove the "also"

Done.

[adamk, 2015-01-27 21:04:33]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2015-01-27 21:05:10]

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

[commit-bot: I haz the power, 2015-01-27 21:06:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-27 21:06:58]

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