Currently we don't support namespace imports or named imports following a default binding.

Patch to update the parser and tests. This turns PNK_IMPORT nodes into ternary nodes, with their second child being an optional namespace import name.

I realised after I posted this that these import specifiers are going to be used to create ImportEntry records for some module, and so rather than having a separate way of returning a namespace import we can just add an import specifier for '*'. This makes things much simpler.

The spec doesn't allow |export *| except as part of |export * from 'module'| so this patch changes that to be an error and adds tests.

I realised I missed a couple things so there's an updated patch that: - parameterises function and class parsing with grammar parameter 'Default' - adds support for |export default| as new parse node kind - adds support for exporting classes - disallows |export *| without following |from|

Comment on attachment 8594001 [details] [diff] [review] update-import-declaration v2 Review of attachment 8594001 [details] [diff] [review]: ----------------------------------------------------------------- bumping review to shu because I suck

Comment on attachment 8596525 [details] [diff] [review] update-export-declaration v2 Review of attachment 8596525 [details] [diff] [review]: ----------------------------------------------------------------- "

Comment on attachment 8594001 [details] [diff] [review] update-import-declaration v2 Review of attachment 8594001 [details] [diff] [review]: ----------------------------------------------------------------- Looks compliant to spec by my reading. ::: js/src/frontend/Parser.cpp @@ +4185,5 @@ > + > + // If the next token is a keyword, the previous call to > + // peekToken matched it as a TOK_NAME, and put it in the > + // lookahead buffer, so this call will match keywords as well. > + MUST_MATCH_TOKEN(TOK_NAME, JSMSG_NO_IMPORT_NAME); So this is kinda sorta abusing the type system. MUST_MATCH_TOKEN returns null(), which for the FullParseHandler is nullptr, so gets automatically coerced to false. That said, it reads nice and I'm fine with nullptr -> false. Could you make this method an explicit FullParseHandler specialization though? I don't see how it would compile for SyntxParseHandler. @@ +4198,5 @@ > + return false; > + if (tt != TOK_NAME) { > + report(ParseError, false, null(), JSMSG_NO_BINDING_NAME); > + return false; > + } Could fold to MUST_MATCH_TOKEN. @@ +4202,5 @@ > + } > + } else { > + // Keywords cannot be bound to themselves, so an import name > + // that is a keyword is a syntax error if it is not followed > + // by the keyword 'as'. Could use a comment pointing to the grammar in ES6. 15.2.2 ImportSpecifier in this case. @@ +4247,5 @@ > + > + if (tt != TOK_NAME) { > + report(ParseError, false, null(), JSMSG_NO_BINDING_NAME); > + return false; > + } Could fold to MUST_MATCH_TOKEN. @@ +4271,5 @@ > +template<> > +bool > +Parser<SyntaxParseHandler>::namedImportsOrNamespaceImport(TokenKind tt, Node importSpecSet) > +{ > + JS_ALWAYS_FALSE(abortIfSyntaxParser()); MOZ_ALWAYS_FALSE

Comment on attachment 8596525 [details] [diff] [review] update-export-declaration v2 Review of attachment 8596525 [details] [diff] [review]: ----------------------------------------------------------------- This is only to update handling |export * from ...| and |export default ...|, right? Everything else was already implemented? ::: js/src/frontend/ParseNode.h @@ +332,5 @@ > * pn_right: PNK_STRING module specifier > + * PNK_EXPORT unary pn_kid: declaration expression > + * PNK_EXPORT_FROM binary pn_left: PNK_EXPORT_SPEC_LIST export specifiers > + * pn_right: PNK_STRING module specifier > + * PNK_EXPORT_DEFAULT unary pn_kid: declaration expression declaration expression here isn't quite right. I don't know what would be better though, without listing the spec. ::: js/src/frontend/Parser.cpp @@ +2592,5 @@ > } > > template <typename ParseHandler> > typename ParseHandler::Node > +Parser<ParseHandler>::functionStmt(DefaultHandling defaultHandling) Could this be fully specialized to FullParseHandler?

(In reply to PTO until 05/25 from comment #9) Thanks for the reviews! > This is only to update handling |export * from ...| and |export default > ...|, right? Everything else was already implemented? Yes those are the only additions needed that I could find. I just noticed that we currently support some syntax that is not in ES6 though so I'll write another patch for that. > > template <typename ParseHandler> > > typename ParseHandler::Node > > +Parser<ParseHandler>::functionStmt(DefaultHandling defaultHandling) > > Could this be fully specialized to FullParseHandler? Did you mean the exportDeclaration() method here?

Patch to remove the |export name| syntax that's not present in ES6 and update the tests. I also added tests to the syntax-error-illegal-character.js test for all the syntax added in the previous patches.

Comment on attachment 8609425 [details] [diff] [review] remove-export-assign-syntax Review of attachment 8609425 [details] [diff] [review]: ----------------------------------------------------------------- Nice.

https://hg.mozilla.org/integration/mozilla-inbound/rev/47804a9979fc https://hg.mozilla.org/integration/mozilla-inbound/rev/5579f3f69a63 https://hg.mozilla.org/integration/mozilla-inbound/rev/edf9b09d1db3

https://hg.mozilla.org/mozilla-central/rev/47804a9979fc https://hg.mozilla.org/mozilla-central/rev/5579f3f69a63 https://hg.mozilla.org/mozilla-central/rev/edf9b09d1db3