https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2373
src/parser.cc:2373: // Create new block with one expected declaration.
Nit: Is the "one" comment still accurate?
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2376
src/parser.cc:2376: decl.nvars = &nvars;
Why not move nvars itself to decl, instead of a pointer?
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2417
src/parser.cc:2417: // Parse initialization expression if present and/or
needed. A
The remainder of this comment mainly explains the definition of
initialization_scope that followed previously and is now gone. You
probably want to move it into PatternMatcher::VisitVariableProxy. Also,
abstracting initialization_scope into a generic helper function seems
inappropriate to me, since it's only meaningful for the specific hack
described by this comment (i.e., it's nonsensical for lexical
declarations' inits, for example).
https://codereview.chromium.org/1130623004/diff/20001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1130623004/diff/20001/src/parser.h#newcode942
src/parser.h:942: class PatternMatcher : private AstVisitor {
Can we call this PatternRewriter? It isn't a "matcher" by itself... And
would it be possible to move the actual declaration to a separate .h
file and just forward-declare it here?
https://codereview.chromium.org/1130623004/diff/20001/src/parser.h#newcode959
src/parser.h:959: return is_const ? declaration_scope : scope;
I think this function is not very meaningful as an abstraction, see
other comment.
https://codereview.chromium.org/1130623004/
--
--
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/d/optout.
