Right now we have (with some handwavery here) template<class ParseHandler, typename CharT> class Parser { ... Node importDeclaration(); ... }; template<> Node Parser<FullParseHandler, char16_t>::importDeclaration() { ...full definition... } template<> Node Parser<SyntaxParseHandler, char16_t>::importDeclaration() { return abortIfSyntaxParser(); } This works to have specialized behavior for full/syntax parsing now. But when we will also have Parser support for single-byte text, we can't just do, e.g. template<typename CharT> Node Parser<FullParseHandler, CharT>::importDeclaration() { ...full definition... } because this is function partial specialization. And C++ no likey. The solution I've reached is this: 1) Rename Parser to GeneralParser, and make it not final. 2) Introduce template<classParseHandler, typename CharT> class Parser; template<typename CharT> class Parser<SyntaxParseHandler, CharT> final : public GeneralParser<SyntaxParseHandler, CharT> { ... Node importDeclaration(); ... }; template<typename CharT> class Parser<FullParseHandler, CharT> final : public GeneralParser<FullParseHandler, CharT> { ... Node importDeclaration(); ... }; 3) Now this *will* work (because class partial specialization *is* permitted, and you're not doing the bad thing to the function): template<typename CharT> Node Parser<FullParseHandler, CharT>::importDeclaration() { ...full definition... } template<typename CharT> Node Parser<SyntaxParseHandler, CharT>::importDeclaration() { return abortIfSyntaxParser(); } Of course there's a bit more trickiness than that -- sometimes a function that *remains* in GeneralParser, wants to call one that lives in the Parser specializations, and other things. But those can be worked around. The workarounds are: GeneralParser functions wanting to call Parser functions: Add a static downcast function to GeneralParser, then add a private inline function in GeneralParser that downcasts and calls accordingly. So for importDeclaration you might have template<class ParseHandler, typename CharT> inline Node GeneralParser<ParseHandler, CharT>::importDeclaration() { return asFinalParser()->importDeclaration(); } Parser wanting to use inherited functions/fields/types: Because GeneralParser<PH, CharT> is a dependent name, every reference to a field/function in it has to be via a dependent name. The solution everyone understands, is to use |this->| everywhere. But that's a big mess, so instead this adds |using Base::tokenStream;| to the two specializations, eliminating the need to touch hundreds of lines of code. The big list of |using|s isn't great, but it's way better than the alternative. Multiple GP<...>::* function overloads with different access rules: You can't |using Base::foo;| if |foo| is private. I forwarded from Parser to GeneralParser in this case using an inline function -- or by renaming one of the underlying overloads. (Note particularly that Parser::innerFunction is now two differently-named functions, which is way way way less confusing to read, entirely independent of this bug's aim.) AutoAwaitIsKeyword: I changed how you invoke AAIK so that it takes GeneralParser instead of Parser, so that users wouldn't have to pass a slightly weird asFinalParser(). That bumps line lengths a bit to squeeze in a GP<...>; it's possible I'll think of a slightly nicer way to skin that cat, but this *does* work now. There are additional simplifications that can be done atop this -- if you specialize Parser::exportDeclaration, for example, then you can move *all* the different export-handling functions from GeneralParser into Parser<FullParseHandler, CharT> and kill off a bunch of dumb GeneralParser<ParseHandler, CharT>::* export functions that just do |asFinalParser()->exportGoop()|. But this is a gonzo patch already, and such is way better deferred than piling on. (I suppose I could have done it before this patch, too -- but I didn't think of it til I was most of the way into this, and the work to flip that is not worth the pain.)

Unfortunately, this patch is more or less not split-uppable. Fortunately, a great deal of the changing is in whitespace or just adding "General" before "Parser". I'll post a -w version after this for additional helpfulness.

This seems like adequate improvement on the AAIK system mentioned in comment 0.

I'll review them tomorrow.

Comment on attachment 8935940 [details] [diff] [review] Patch Review of attachment 8935940 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/Parser.h @@ +1135,5 @@ > + bool checkExportedNameForFunction(Node node); > + bool checkExportedNameForClass(Node node); > + inline bool checkExportedNameForClause(Node node); > + > + bool trySyntaxParseInnerFunction(Node pn, HandleFunction fun, uint32_t toStringStart, both "pn" and "node" are used for genera/syntax/full parsers, in declaration and definition. it might be nice to align to either. @@ +1141,5 @@ > + FunctionSyntaxKind kind, GeneratorKind generatorKind, > + FunctionAsyncKind asyncKind, bool tryAnnexB, > + Directives inheritedDirectives, Directives* newDirectives); > + > + bool skipLazyInnerFunction(Node node, uint32_t toStringStart, FunctionSyntaxKind kind, same here (pn vs node).

Better than |node|, I went with |funcNode| so as to clarify just what node it is at various use sites (especially when |node| would be forwarded in a big garbage-list argument set and it'd be not clear just what it was). Made it a separate patch-atop rather than touch the mega-patch here with that mostly cosmetic change. I expect to land this pretty much Any Day Now, along with several other bugs and changes, as one dozen-rev landing, once I get two last reviews from :Yoric in another bug.

Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/mozilla-inbound/rev/9f31a702d5de Rename Parser to GeneralParser, and move all partially-specialized (by ParseHandler) functions in GeneralParser into a new Parser class inheriting from GeneralParser. This permits these functions to be specialized without having two identical definitions when CharT isn't solely char16_t. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/49a259667582 Change the name of some |Node pn| in function-parsing functions to |Node funcNode|. r=arai https://hg.mozilla.org/integration/mozilla-inbound/rev/bdb40755d36a Adjust AutoAwaitIsKeyword template parameters slightly so a full parser type need not be written out. r=arai

https://hg.mozilla.org/mozilla-central/rev/9f31a702d5de https://hg.mozilla.org/mozilla-central/rev/49a259667582 https://hg.mozilla.org/mozilla-central/rev/bdb40755d36a