Thank you for review. I uploaded new patch set.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode2858
src/parser.cc:2858: // loop is executed to update the loop variables.
The the loop condition is
On 2014/05/22 12:55:44, rossberg wrote:
Nit: "The the"
Done.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode2886
src/parser.cc:2886: // }
On 2014/05/22 12:55:44, rossberg wrote:
So, eventually, there should be a comment here along the lines of:
"Similarly for other mutable lexical declarations (e.g. function).
Immutable
declarations (like const) do not need to be rewritten."
As discussed offline, only lets and consts are allowed here.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode2926
src/parser.cc:2926: Statement* body = ParseStatement(NULL, CHECK_OK);
On 2014/05/22 12:55:44, rossberg wrote:
It's somewhat inappropriate to do actual parsing in a function called
Desugar. I
would pass this in as an argument.
Done.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode3008
src/parser.cc:3008: ZoneStringList let_bindings(1, zone());
On 2014/05/22 12:55:44, rossberg wrote:
lexical_bindings
Leaving as is.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode3164
src/parser.cc:3164: // If there are let bindings, then condition and the
next statement of the
On 2014/05/22 12:55:44, rossberg wrote:
"let bindings" -> "lexical declarations"
Leaving as is.
https://codereview.chromium.org/292743009/diff/60001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/292743009/diff/60001/src/parser.h#newcode721
src/parser.h:721: Statement* DesugarLetBindingsInForStatement(
On 2014/05/22 12:55:44, rossberg wrote:
Nit: DesugarLexicalDeclarationInForStatement, since in fact, it might
also be
const (or others).
As discussed offline, const and others are not handled here.
https://codereview.chromium.org/292743009/diff/60001/src/parser.h#newcode724
src/parser.h:724: Statement* Assign(Token::Value op, VariableProxy* dst,
Expression* src,
On 2014/05/22 12:55:44, rossberg wrote:
I am not sure if it's actually worth introducing these extra
functions. But if
you do, they should probably be named NewAssignmentStatement and
NewTemporary
for consistency with other such helpers. Also, please then update
other places
in the parser calling NewAssignmentExpression and Scope::NewTemporary
to use
them consistently.
Done. Inlined the functions to be consistent.
https://codereview.chromium.org/292743009/
--
--
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.
