https://codereview.chromium.org/1259283002/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1259283002/diff/1/src/parser.cc#newcode1205
src/parser.cc:1205: ParseFormalParameter(is_rest, &formals,
&formals_classifier, &ok);
Check ok before declaring?
https://codereview.chromium.org/1259283002/diff/1/src/parser.cc#newcode2222
src/parser.cc:2222: !scope_->is_declaration_scope() ? LET : VAR;
This looks unrelated to the rest of the change; maybe it's further fixup
from the broadening of declaration scope definition?
https://codereview.chromium.org/1259283002/diff/1/src/parser.cc#newcode3917
src/parser.cc:3917: for (int i = 0; i < parameters->arity; ++i) {
Seems scary to use arity for this instead of the params. As in the other
change, this separation of arity handling from the actual list of
parameters feels brittle, and I'd feel better if there were something
other than a DCHECK keeping the arity and params.size() in lock-step.
https://codereview.chromium.org/1259283002/diff/1/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1259283002/diff/1/src/parser.h#newcode559
src/parser.h:559: DCHECK(arity == params.length());
This code could be responsible for keeping arity in sync for the Parser
version, letting the preparser handle the incrementing in
AddFormalParameter (see my comment in preparser.h).
A more robust alternative: make the |arity| member private in the
superclass (arity_), and add an arity() getter which in the PreParser
class returns that private member and in this version returns
params.length().
https://codereview.chromium.org/1259283002/diff/1/src/parser.h#newcode1331
src/parser.h:1331: DCHECK(parameters->arity ==
parameters->params.length());
Nit: DCHECK_EQ
https://codereview.chromium.org/1259283002/diff/1/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1259283002/diff/1/src/preparser.h#newcode1644
src/preparser.h:1644: bool is_rest) {}
One option that would make this thing slightly less brittle is if this
implementation incremented the arity, and the Parser one did the same.
At the least, it would make the Parser implementation a bit
better-factored.
https://codereview.chromium.org/1259283002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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 v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
