Reviewers: adamk,
Description:
[es6] Correct length for functions with default parameters
[email protected]
BUG=v8:2160
LOG=N
Please review this at https://codereview.chromium.org/1311163002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+43, -27 lines):
M src/parser.h
M src/scopes.h
M src/scopes.cc
M test/mjsunit/harmony/default-parameters.js
M test/mjsunit/harmony/destructuring.js
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
355778b042d3368b3f340cc58f08a357bc7879a4..8713d1b331cd6b2d48771a11c1a8951cb964b0d2
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -1341,8 +1341,9 @@ void ParserTraits::DeclareFormalParameter(
auto name = is_simple || parameter.is_rest
? parameter.name : parser_->ast_value_factory()->empty_string();
auto mode = is_simple || parameter.is_rest ? VAR : TEMPORARY;
- Variable* var =
- scope->DeclareParameter(name, mode, parameter.is_rest,
&is_duplicate);
+ Variable* var = scope->DeclareParameter(
+ name, mode, parameter.initializer != nullptr, parameter.is_rest,
+ &is_duplicate);
if (is_duplicate) {
classifier->RecordDuplicateFormalParameterError(
parser_->scanner()->location());
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
9a6e3aae6da4d72b705970990826e3fd78308f90..8c99044e72b57a659ad49953331d04e129d62dac
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -180,6 +180,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope*
outer_scope,
num_global_slots_ = 0;
num_modules_ = 0;
module_var_ = NULL;
+ arity_ = 0;
has_simple_parameters_ = true;
rest_parameter_ = NULL;
rest_index_ = -1;
@@ -465,10 +466,12 @@ Variable* Scope::Lookup(const AstRawString* name) {
}
-Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode
mode,
- bool is_rest, bool* is_duplicate) {
+Variable* Scope::DeclareParameter(
+ const AstRawString* name, VariableMode mode,
+ bool is_optional, bool is_rest, bool* is_duplicate) {
DCHECK(!already_resolved());
DCHECK(is_function_scope());
+ DCHECK(!is_optional || !is_rest);
Variable* var;
if (mode == TEMPORARY) {
var = NewTemporary(name);
@@ -479,6 +482,9 @@ Variable* Scope::DeclareParameter(const AstRawString*
name, VariableMode mode,
// TODO(wingo): Avoid O(n^2) check.
*is_duplicate = IsDeclaredParameter(name);
}
+ if (!is_optional && !is_rest && arity_ == params_.length()) {
+ ++arity_;
+ }
if (is_rest) {
DCHECK_NULL(rest_parameter_);
rest_parameter_ = var;
Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index
4990fab994574a9fb9fbb27814ba93faf3ad5c86..28cc742eb25ac0c1a2a9247784e653aca5e50588
100644
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -128,8 +128,9 @@ class Scope: public ZoneObject {
// Declare a parameter in this scope. When there are duplicated
// parameters the rightmost one 'wins'. However, the implementation
// expects all parameters to be declared and from left to right.
- Variable* DeclareParameter(const AstRawString* name, VariableMode mode,
- bool is_rest, bool* is_duplicate);
+ Variable* DeclareParameter(
+ const AstRawString* name, VariableMode mode,
+ bool is_optional, bool is_rest, bool* is_duplicate);
// Declare a local variable in this scope. If the variable has been
// declared before, the previously declared variable is returned.
@@ -365,16 +366,8 @@ class Scope: public ZoneObject {
return params_[index];
}
- // Returns the default function arity --- does not include rest
parameters.
- int default_function_length() const {
- int count = params_.length();
- if (rest_index_ >= 0) {
- DCHECK(count > 0);
- DCHECK(is_function_scope());
- --count;
- }
- return count;
- }
+ // Returns the default function arity excluding default or rest
parameters.
+ int default_function_length() const { return arity_; }
int num_parameters() const { return params_.length(); }
@@ -632,6 +625,7 @@ class Scope: public ZoneObject {
Variable* module_var_;
// Info about the parameter list of a function.
+ int arity_;
bool has_simple_parameters_;
Variable* rest_parameter_;
int rest_index_;
Index: test/mjsunit/harmony/default-parameters.js
diff --git a/test/mjsunit/harmony/default-parameters.js
b/test/mjsunit/harmony/default-parameters.js
index
3be5916d9bf5553265889489f2b143b51cdc2928..0d87b0371217876779656f6a32568c065cecd365
100644
--- a/test/mjsunit/harmony/default-parameters.js
+++ b/test/mjsunit/harmony/default-parameters.js
@@ -418,15 +418,14 @@
(function TestFunctionLength() {
- // TODO(rossberg): Fix arity.
- // assertEquals(0, (function(x = 1) {}).length);
- // assertEquals(0, (function(x = 1, ...a) {}).length);
- // assertEquals(1, (function(x, y = 1) {}).length);
- // assertEquals(1, (function(x, y = 1, ...a) {}).length);
- // assertEquals(2, (function(x, y, z = 1) {}).length);
- // assertEquals(2, (function(x, y, z = 1, ...a) {}).length);
- // assertEquals(1, (function(x, y = 1, z) {}).length);
- // assertEquals(1, (function(x, y = 1, z, ...a) {}).length);
- // assertEquals(1, (function(x, y = 1, z, v = 2) {}).length);
- // assertEquals(1, (function(x, y = 1, z, v = 2, ...a) {}).length);
+ assertEquals(0, (function(x = 1) {}).length);
+ assertEquals(0, (function(x = 1, ...a) {}).length);
+ assertEquals(1, (function(x, y = 1) {}).length);
+ assertEquals(1, (function(x, y = 1, ...a) {}).length);
+ assertEquals(2, (function(x, y, z = 1) {}).length);
+ assertEquals(2, (function(x, y, z = 1, ...a) {}).length);
+ assertEquals(1, (function(x, y = 1, z) {}).length);
+ assertEquals(1, (function(x, y = 1, z, ...a) {}).length);
+ assertEquals(1, (function(x, y = 1, z, v = 2) {}).length);
+ assertEquals(1, (function(x, y = 1, z, v = 2, ...a) {}).length);
})();
Index: test/mjsunit/harmony/destructuring.js
diff --git a/test/mjsunit/harmony/destructuring.js
b/test/mjsunit/harmony/destructuring.js
index
69e144b26fd2a190cc233ef70c49f6eb12f04234..84eeb893f0a08aeb9f3191a275952e21c48a9bba
100644
--- a/test/mjsunit/harmony/destructuring.js
+++ b/test/mjsunit/harmony/destructuring.js
@@ -953,3 +953,19 @@
assertThrows("'use strict'; let x = {}; for (let [x, y] in {x});",
ReferenceError);
assertThrows("'use strict'; let x = {}; for (let [y, x] in {x});",
ReferenceError);
}());
+
+
+(function TestFunctionLength() {
+ assertEquals(1, (function({}) {}).length);
+ assertEquals(1, (function([]) {}).length);
+ assertEquals(1, (function({x}) {}).length);
+ assertEquals(1, (function({}, ...a) {}).length);
+ assertEquals(1, (function({x}, {y} = {}) {}).length);
+ assertEquals(1, (function({x}, {y} = {}, ...a) {}).length);
+ assertEquals(2, (function(x, {y}, {z} = {}) {}).length);
+ assertEquals(2, (function({x}, {}, {z} = {}, ...a) {}).length);
+ assertEquals(1, (function(x, {y} = {}, {z}) {}).length);
+ assertEquals(1, (function({x}, {y} = {}, {z}, ...a) {}).length);
+ assertEquals(1, (function(x, {y} = {}, {z}, {v} = {}) {}).length);
+ assertEquals(1, (function({x}, {y} = {}, {z}, {v} = {}, ...a)
{}).length);
+})();
--
--
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.
