Readiing the CL I realised that our parsing of for-loops is not quite
correct
yet: we do not yet allow arbitrary lexical declarations as init statements,
like
the spec prescribes.
Also, I think the logic should ignore immutable variables in the init, since
they don't require copying.
But that's stuff for separate CLs.
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
Nit: "The the"
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode2886
src/parser.cc:2886: // }
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."
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode2926
src/parser.cc:2926: Statement* body = ParseStatement(NULL, CHECK_OK);
It's somewhat inappropriate to do actual parsing in a function called
Desugar. I would pass this in as an argument.
https://codereview.chromium.org/292743009/diff/60001/src/parser.cc#newcode3008
src/parser.cc:3008: ZoneStringList let_bindings(1, zone());
lexical_bindings
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
"let bindings" -> "lexical declarations"
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(
Nit: DesugarLexicalDeclarationInForStatement, since in fact, it might
also be const (or others).
https://codereview.chromium.org/292743009/diff/60001/src/parser.h#newcode724
src/parser.h:724: Statement* Assign(Token::Value op, VariableProxy* dst,
Expression* src,
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.
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.
