Teach ModuleDescriptor about basic local exports

Teach ModuleDescriptor about basic local exports

Add() becomes AddLocalExport, which takes an export_name and a local_name.
New parsing tests exercise this.

Also start generating exports for default exports (though this doesn't yet
handle anonymous default exports).

BUG=v8:1569
LOG=n

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

[adamk, 2015-02-19 00:45:24]

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

[adamk, 2015-02-19 00:51:32]

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

[adamk, 2015-02-19 00:51:33]

The other obvious approach here would be to retrofit
ExportDeclaration/Parser::Declare to do this work. +marja for
Parser-code-factoring opinions.

[adamk, 2015-02-19 00:58:36]

To talk to myself a little bit more: my initial reasoning behind doing this in
ModuleDescriptor instead of as a Declaration node is that I don't foresee much
advantage of having exports in the AST. Are there any visitors where
VisitExportDeclaration would have an implementation?

[marja, 2015-02-19 10:08:52]

Parsing LGTM.

In general I'd like to move Parser::Declare to ParserBase and make PreParser
declare identifiers, too, but I'm not sure whether that'll ever be feasible. If
PreParser is not going to understand modules, makes sense to keep
Parser::Declare as simple as possible and not add module stuff there.

[arv (Not doing code reviews), 2015-02-19 15:05:44]

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

[arv (Not doing code reviews), 2015-02-19 15:05:44]

LGTM

https://codereview.chromium.org/934323004/diff/1/src/messages.js
File src/messages.js (right):

https://codereview.chromium.org/934323004/diff/1/src/messages.js#newcode179
src/messages.js:179: duplicate_export:              ["Cannot export '", "%0", "'
more than once"],
I think the error message should contain "duplicate". How about?

"Duplicate export of '", %0, "'"

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

https://codereview.chromium.org/934323004/diff/1/src/parser.cc#newcode1589
src/parser.cc:1589: // TODO(adamk): Possibly report this error at the right
place.
Maybe pass one more list with the locations to ParseExportClause.

[rossberg, 2015-02-19 15:18:13]

> To talk to myself a little bit more: my initial reasoning behind doing this in
> ModuleDescriptor instead of as a Declaration node is that I don't foresee much
> advantage of having exports in the AST.

Yes, I wasn't sure either at the time, as you can tell from some TODOs.

However, once we start with types, we might want them in the AST.

https://codereview.chromium.org/934323004/diff/1/src/messages.js
File src/messages.js (right):

https://codereview.chromium.org/934323004/diff/1/src/messages.js#newcode179
src/messages.js:179: duplicate_export:              ["Cannot export '", "%0", "'
more than once"],
On 2015/02/19 15:05:44, arv wrote:
> I think the error message should contain "duplicate". How about?
> 
> "Duplicate export of '", %0, "'"

+1

[adamk, 2015-02-19 19:09:48]

New patchsets have been uploaded after l-g-t-m from
marja@chromium.org,arv@chromium.org

[adamk, 2015-02-19 19:10:10]

https://codereview.chromium.org/934323004/diff/1/src/messages.js
File src/messages.js (right):

https://codereview.chromium.org/934323004/diff/1/src/messages.js#newcode179
src/messages.js:179: duplicate_export:              ["Cannot export '", "%0", "'
more than once"],
On 2015/02/19 15:05:44, arv wrote:
> I think the error message should contain "duplicate". How about?
> 
> "Duplicate export of '", %0, "'"

Done.

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

https://codereview.chromium.org/934323004/diff/1/src/parser.cc#newcode1589
src/parser.cc:1589: // TODO(adamk): Possibly report this error at the right
place.
On 2015/02/19 15:05:44, arv wrote:
> Maybe pass one more list with the locations to ParseExportClause.

Hmm. There will be cases, with "export * from ...", where we'll end up with
export conflicts at the linking stage, and this won't help provide better error
messages in those cases. But I expect "export * from" to be one of the last
things I do, so I can refactor as needed when I get around to that. It's looking
like it would be useful to have the locations of each export available later in
several places.

Added a list of locations for now, I think there's more refactoring I can do
here. Also added tests for these error messages (to help with refactoring
later).

[arv (Not doing code reviews), 2015-02-19 19:27:42]

LGTM

https://codereview.chromium.org/934323004/diff/20001/test/message/export-dupl...
File test/message/export-duplicate.out (right):

https://codereview.chromium.org/934323004/diff/20001/test/message/export-dupl...
test/message/export-duplicate.out:5: export { a, b };
Hot!

[adamk, 2015-02-19 19:31:44]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2015-02-19 19:32:08]

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

[commit-bot: I haz the power, 2015-02-19 20:15:02]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-19 20:15:26]

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