On 2014/07/01 at 07:23:40, marja wrote:
I don't think we need to make arrow functions lazy. They're small, so
switching to PreParser for the function body is not going to make a
difference.
Let's keep it simple and not do lazy arrow functions.
This sounds reasonable at first: for most arrow functions one certainly
would
expect small
bodies. But then it is very common for callback functions in Node.js
programs to
end up
having big bodies, and those functions are candidates to be written as arrow
functions...
WDYT? Should I go ahead and remove the bits for lazy parsing of arrow
functions?
For the
moment I am slighly in favor of keeping lazy arrow functions.
https://codereview.chromium.org/160073006/diff/580001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/160073006/diff/580001/src/parser.cc#newcode3322
src/parser.cc:3322: Vector<VariableProxy*>
ParserTraits::ParameterListFromExpression(
Comment from the previous round:
Here you have the same logic in 2 functions; would it be possible to have
only
one function which both checks if the expression is a parameter list and
constructs the vector?
I am going to put both back in the same function.
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode893
src/preparser.h:893: // TODO(aperezdc): Would allowing lazy compilation in
preparser make sense?
Doesn't seem very sensemaking to me, when we are in PreParser, we're
already
parsing a lazy function and cannot get lazier than that :)
Haha. That is what I thought it would be, so I had already written "return
false"
from the first versions of the CL :-)
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode899
src/preparser.h:899: bool IsDeclared(const PreParserIdentifier&
identifier)
const {
What cases does this IsDeclared actually detect? Pls add a comment.
This is a leftover from the previous parameter checking approach. Right now
the
PreParser never calls this because
PreParserTraits::ParameterListFromExpression()
always returns an empty Vector. I am removing this and making IsDeclared()
and
DeclareParameter() just dummies.
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode1274
src/preparser.h:1274: bool
IsValidArrowFunctionParameterList(PreParserExpression expression) {
Pls reorder these functions so that they're in the same order in
ParserTraits
and PreParserTraits. (So this Is... function should go where the other IsFoo
functions are).
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode1280
src/preparser.h:1280: void CheckConflictingVarDeclarations(
Nit: formatting.
You'd probably want to have a look how you can use clang-format in your
favourite editor :)
It turns out that clang-format comes now with Vim support. Thanks for
pointing
it out, I have gone without checking that for a long while, it seems.
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode2539
src/preparser.h:2539:
this->CheckStrictFunctionNameAndParameters(this->EmptyIdentifier(),
This is not inside if (strict_mode() == STRICT) because arrow function
parameter lists are always strict, right? Pls add a comment.
Exactly. I will add a comment here.
https://codereview.chromium.org/160073006/diff/580001/src/preparser.h#newcode2557
src/preparser.h:2557: return this->EmptyExpression();
This will change when you start generating the arrow function AST for
real,
right? Pls add a comment.
Exactly.
https://codereview.chromium.org/160073006/diff/580001/test/cctest/test-parsing.cc#newcode1457
test/cctest/test-parsing.cc:1457:
This file seems to contain some unintentional changes still..
All should be gone in the next update.
https://codereview.chromium.org/160073006/diff/580001/test/cctest/test-parsing.cc#newcode2817
test/cctest/test-parsing.cc:2817: TEST(ErrorsArrowFunctions) {
I like this test! :)
:)
https://codereview.chromium.org/160073006/
--
--
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.
