On 2014/09/10 06:33:56, rossberg wrote:
https://codereview.chromium.org/422923004/diff/60001/src/preparser.h#newcode613
src/preparser.h:613: bool IsArguments(const AstValueFactory*) const {
Why is this change necessary when the argument isn't even used?
This is to make the prototypes of "Parser::IsArguments()" and
"PreParser::IsArguments()" have the same arguments. The call
site is a template which gets expanded in both of "Parser"
and "PreParser".
https://codereview.chromium.org/422923004/diff/60001/src/preparser.h#newcode831
src/preparser.h:831: if (ident.IsEval() || ident.IsArguments(nullptr) ||
ident.IsYield() ||
Unfortunately, we can't use C++11 yet...
Fixed. My brain subconsciously keeps insisting in C+11 being the
way to go... ;-)
https://codereview.chromium.org/422923004/diff/60001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/422923004/diff/60001/src/scopes.cc#newcode1175
src/scopes.cc:1175: if (inner->scope_uses_this_ ||
inner->inner_scope_uses_this_) {
If inner is a function scope and not an arrow function, then both new
flags
must
not be propagated.
Done. To be able to implement this I had to go ahead and add the
"ARROW_SCOPE" scope type. For the sake of keeping things simple,
I have made "Scope::is_function_scope()" return true for both
normal functions and arrow functions. To tell apart a normal
function from an arrow, one has to use "Scope::is_arrow_scope()".
I think this matches the conceptual idea that arrow functions
are functions -- of a particular kind, but functions nevertheless.
Now, parsing an arrow function effectively returns a scope of
this new "ARROW_SCOPE" type, and a couple of TODO items are now
gone.
https://codereview.chromium.org/422923004/diff/60001/test/cctest/test-parsing.cc#newcode950
test/cctest/test-parsing.cc:950: { "var x = function () { this.foo = 42
};",
On 2014/09/04 16:41:21, arv wrote:
> Why do we need to track the this reference inside a function expression?
Indeed, this seems wrong. Although it should be true for an arrow
function.
I am not sure how to proceed WRT this. Now that propagation of
the "inner_uses_$foo" flags does not go outside normal function
scopes, it is clearly something that pertains only to arrow
functions... so how about renaming to "arrow_inner_uses_$foo"?
That would better indicate that the flag is relevant only for
arrow functions.
https://codereview.chromium.org/422923004/diff/60001/test/cctest/test-parsing.cc#newcode968
test/cctest/test-parsing.cc:968: };
We probably need some tests with block scopes as well, e.g.
"while (true) { let x; this, arguments; }" => false, false, true, true
I have now added this, and a couple more.
https://codereview.chromium.org/422923004/
--
--
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.
