Great incremental first step
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc#newcode3538
src/parser.cc:3538:
How about adding a comment block explaining how the desugaring is done?
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc#newcode3561
src/parser.cc:3561: Runtime::FunctionForId(Runtime::kInlineArguments);
I think Andreas wanted:
function f(x = expr) {}
to be desugared to:
function f(temp) {
let x = IS_UNDEFINED(temp) ? expr : temp;
}
If you use %_Arguments(i), do you also need to check the number of
arguments so you do not index out of bounds?
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc#newcode3612
src/parser.cc:3612: } else {
no else after return
https://codereview.chromium.org/1127063003/diff/1/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1127063003/diff/1/src/parser.h#newcode752
src/parser.h:752: scope->DeclareParameter(name, VAR, pos, is_rest,
&is_duplicate);
keep pos at end?
https://codereview.chromium.org/1127063003/diff/1/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/1/src/preparser.h#newcode3535
src/preparser.h:3535: Scope* param_scope = NewScope(scope_,
BLOCK_SCOPE);
Is BLOCK_SCOPE sufficient? Does it need this new VAR_DECLARATION_SCOPE?
function f(x = eval('var z = 2'), y = z) {
return y;
}
f() === 2 // ?
is that eval a strict eval? So confusing...
https://codereview.chromium.org/1127063003/diff/1/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/1/src/scopes.cc#newcode513
src/scopes.cc:513: void Scope::ShadowParametersForExpressions() {
This could use a comment
https://codereview.chromium.org/1127063003/diff/1/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1127063003/diff/1/src/scopes.h#newcode129
src/scopes.h:129: int pos, bool is_rest, bool* is_duplicate);
keep pos at end for consistency
https://codereview.chromium.org/1127063003/diff/1/test/mjsunit/harmony/optional-arguments.js
File test/mjsunit/harmony/optional-arguments.js (right):
https://codereview.chromium.org/1127063003/diff/1/test/mjsunit/harmony/optional-arguments.js#newcode17
test/mjsunit/harmony/optional-arguments.js:17: // TODO(caitp): infer
function name correctly
https://code.google.com/p/v8/issues/detail?id=3699
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.
