[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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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);
+})();