Reviewers: Michael Starzinger,
Message:
mstarzinger: Thanks for having a look!
Do you like this version better?
https://codereview.chromium.org/149913006/diff/100001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/149913006/diff/100001/src/parser.cc#newcode607
src/parser.cc:607: new ParserTraits(this)),
On 2014/02/10 13:28:03, Michael Starzinger wrote:
If we make the ParserBase inherit from ParserTraits/PreParserTraits
then we
wouldn't need to allocate any additional object, everything needed to
parse
would be embedded in the Parser objects.
Done.
https://codereview.chromium.org/149913006/diff/100001/src/parser.cc#newcode1098
src/parser.cc:1098: traits_->ReportMessage("module_export_undefined",
On 2014/02/10 13:28:03, Michael Starzinger wrote:
Likewise calling ReportMessage() wouldn't need an explicit receiver if
ParserBase inherits from ParserTraits.
Done.
https://codereview.chromium.org/149913006/diff/100001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/149913006/diff/100001/src/parser.h#newcode411
src/parser.h:411: class Parser;
On 2014/02/10 13:28:03, Michael Starzinger wrote:
nit: Can we group this forward declaration together with the previous
one and
alpha-sort?
Done.
https://codereview.chromium.org/149913006/diff/100001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/149913006/diff/100001/src/preparser.h#newcode52
src/preparser.h:52: delete traits_;
On 2014/02/10 13:28:03, Michael Starzinger wrote:
It feels dangerous to have the ownership of the Traits object be
transferred to
the parser. Could we avoid that?
Done. but this is not pretty either:
1) Functions in preparser.h cannot just say GetSymbol() etc. ("there are
no arguments to nnn that depend on a template parameter, so a
declaration of mmm must be available")
2) Functions in parser.cc can say GetSymbol() etc., but all functions
will eventually move to ParserBase, so, it's not much use.
3) ReportMessage exists on multiple levels of the class hierarchy with
different parameters, so, to get the right version called, I need to
qualify with the class.
Description:
Traitify ParserBase and move functions there.
The long-term goal is to move all recursive descent functions from Parser
and
PreParser into ParserBase, but first they need to be unified.
Notes:
- The functions moved in this CL: ParseIdentifier, ParseIdentifierName,
ParseIdentifierNameOrGetOrSet, ParseIdentifierOrStrictReservedWord.
- IOW, this CL removes Parser::ParseIdentifier and
PreParser::ParseIdentifier
and adds ParserBase::ParseIdentifier, etc.
- Error reporting used to require virtual funcs; now error reporting is
moved to
the Traits too, and ParserBase no longer needs to be virtual.
- I had to move PreParser::Identifier out of the PreParser class, because
otherwise PreParserTraits cannot use it in a typedef.
BUG=v8:3126
LOG=N
Please review this at https://codereview.chromium.org/149913006/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+646, -673 lines):
M src/parser.h
M src/parser.cc
M src/preparser.h
M src/preparser.cc
M test/cctest/test-parsing.cc
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
