From: rossberg Date: Tue, 25 Aug 2015 18:51:57 +0000 (-0700) Subject: [es6] Correct length for functions with default parameters X-Git-Tag: upstream/4.7.83~679 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0b3b72671e62234dffd75c909fcb77c8bd60c7fc;p=platform%2Fupstream%2Fv8.git [es6] Correct length for functions with default parameters R=adamk@chromium.org BUG=v8:2160 LOG=N Review URL: https://codereview.chromium.org/1311163002 Cr-Commit-Position: refs/heads/master@{#30361} --- diff --git a/src/parser.h b/src/parser.h index dced211..05daabb 100644 --- a/src/parser.h +++ b/src/parser.h @@ -1338,8 +1338,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); + bool is_optional = parameter.initializer != nullptr; + Variable* var = scope->DeclareParameter( + name, mode, is_optional, parameter.is_rest, &is_duplicate); if (is_duplicate) { classifier->RecordDuplicateFormalParameterError( parser_->scanner()->location()); diff --git a/src/scopes.cc b/src/scopes.cc index 9a6e3aa..8c99044 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; diff --git a/src/scopes.h b/src/scopes.h index 97606a7..4c36938 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. @@ -369,16 +370,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(); } @@ -636,6 +629,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_; diff --git a/test/mjsunit/harmony/default-parameters.js b/test/mjsunit/harmony/default-parameters.js index bd9cb42..fdc4e60 100644 --- a/test/mjsunit/harmony/default-parameters.js +++ b/test/mjsunit/harmony/default-parameters.js @@ -362,15 +362,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); })(); diff --git a/test/mjsunit/harmony/destructuring.js b/test/mjsunit/harmony/destructuring.js index 69e144b..84eeb89 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); +})();