LGTM, just some nits left.
https://codereview.chromium.org/149913006/diff/150001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/149913006/diff/150001/src/parser.cc#newcode4084
src/parser.cc:4084: if (!eval_args_error_log.IsValid() &&
nit: Should fit into one line again now.
https://codereview.chromium.org/149913006/diff/150001/src/parser.cc#newcode4435
src/parser.cc:4435: if (lhs != NULL && !lhs->is_this() &&
nit: Likewise.
https://codereview.chromium.org/149913006/diff/150001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/149913006/diff/150001/src/preparser.h#newcode53
src/preparser.h:53: ~ParserBase() {}
nit: Shouldn't be necessary to declare at all now.
https://codereview.chromium.org/149913006/diff/150001/src/preparser.h#newcode216
src/preparser.h:216: typename Traits::IdentifierType ParseIdentifier(
Puzzler of the day: Can a typename nested within a class be forward
declared? Would spare us from repeating the nasty "typename" everywhere.
Chances are slim I guess, but lets try. :)
https://codereview.chromium.org/149913006/diff/150001/src/preparser.h#newcode307
src/preparser.h:307: // it is used) are generally omitted.
This whole block of comments mainly applies to the PreParser class, can
we move it down to right before the PreParser class?
https://codereview.chromium.org/149913006/
--
--
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.
