https://codereview.chromium.org/1127063003/diff/100001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/100001/src/parser.cc#newcode3575
src/parser.cc:3575: proxy->var()->set_maybe_assigned();
On 2015/05/12 14:04:53, rossberg wrote:
Why is this set?
Ping
https://codereview.chromium.org/1127063003/diff/100001/src/parser.cc#newcode4301
src/parser.cc:4301: body->AddAll(*inner_body, zone());
On 2015/05/12 14:34:17, caitp wrote:
On 2015/05/12 14:04:53, rossberg wrote:
> Shouldn't this create a block statement instead?
Is this for the extra bailout id? I have mostly been thinking about
scoping, so
I'm not sure what effect wrapping it in a block statement has
The scope chain needs to be in sync with the context chain we create at
runtime (up to some optimisations). I'm not sure how it can work
without, at least not if something like eval or with appears in the
body, referring to respective variables (which reminds me that we should
probably have tests for those cases :) ).
https://codereview.chromium.org/1127063003/diff/100001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/100001/src/scopes.cc#newcode513
src/scopes.cc:513: void Scope::ShadowParametersForExpressions() {
On 2015/05/12 14:34:17, caitp wrote:
On 2015/05/12 14:04:53, rossberg wrote:
> Instead of spreading this out into a different function, can't we do
that
during
> desugaring, right before invoking re-Declare on the respective
variable in
> DesugarInitializedPArameters?
well the scope VariableMap is private, so it's hard for the parser to
do itself
--- do you mean just get rid of the parser version of the function?
Well, this whole removing variables from scope thing still worries me.
Do you think it would be possible to reuse the same variables in the
desugaring instead of removing and re-Declare-ing them? I figure you'd
still need to do some adjustments (e.g. modifying their VariableMode),
but those may be less drastic.
https://codereview.chromium.org/1127063003/diff/100001/test/mjsunit/harmony/optional-arguments.js
File test/mjsunit/harmony/optional-arguments.js (right):
https://codereview.chromium.org/1127063003/diff/100001/test/mjsunit/harmony/optional-arguments.js#newcode55
test/mjsunit/harmony/optional-arguments.js:55:
On 2015/05/12 18:35:35, caitp wrote:
On 2015/05/12 14:04:53, rossberg wrote:
> More tests, e.g.:
>
> - (function(x = this) { return x }).call(o) === o
> - (function(x = () => this) { return x() }).call(o) === o
> - (function f(x, y = f) { return x ? y(false) + 1 : 0 })(true) === 1
> - (function(x, y = function() { return x }) { return y() })(7) === 7
> - (function(x = function() { return y }, y) { return x() })(8) === 8
> - (function(x, y = eval("x")) { let x; return y })(7) === 7
> - (function(x = eval("y"), y = 0) {})() throws
> - (function(x, y = () => eval("x")) { let x; return y() })(7) === 7
> - (function(x = () => eval("y"), y = 9) { let y; return x() })() ===
9
> - (function(x = {a: 4}) { return x.a })() === 4
> - (function(x, y = {a: x}) { return y.a })(5) === 5
> - a function large enough to trigger lazy parsing
Please add a couple of lazy parsing tests as well (you need to write
global functions with >2K characters size; long dummy comments are good
enough to achieve that).
Added eval and arrow variants to a bunch of these tests --- something
is very
broken with the TDZ behaviour when "eval" is used, I'm not sure if
this affects
normal lexical variables or not. If it does I'll file a bug
How do these cases fail? If it is due to a deeper bug then it should be
possible to create a JS test in desugared form that exhibits it. If not,
then
s.th likely is wrong with the scope handling in this CL, and probably
should be
fixed before landing.
https://codereview.chromium.org/1127063003/
--
--
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.
