PTAL - comments addressed
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.
On 2015/05/11 11:49:33, rossberg wrote:
Nit: Is the "one" comment still accurate?
Yes - we still expect 1 declaration in the majority of cases.
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2376
src/parser.cc:2376: decl.nvars = &nvars;
On 2015/05/11 11:49:33, rossberg wrote:
Why not move nvars itself to decl, instead of a pointer?
I want to keep decl_ const in PatternRewriter.
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2417
src/parser.cc:2417: // Parse initialization expression if present and/or
needed. A
On 2015/05/11 11:49:33, rossberg wrote:
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).
Done.
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 {
On 2015/05/11 11:49:33, rossberg wrote:
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?
Oh, good trick - I didn't realize it is allowed in C++!
Done
https://codereview.chromium.org/1130623004/diff/20001/src/parser.h#newcode959
src/parser.h:959: return is_const ? declaration_scope : scope;
On 2015/05/11 11:49:33, rossberg wrote:
I think this function is not very meaningful as an abstraction, see
other
comment.
Done.
https://codereview.chromium.org/1130623004/
--
--
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.
