[destructuring] Implement parameter pattern matching.

Scoping for initializers is yet incorrect. Defaults are not supported.

R=arv@chromium.org,rossberg@chromium.org
BUG=v8:811
LOG=N

Committed: https://crrev.com/42f30f4ded2b1ca0c4caa7639e6206e93c78ee70
Cr-Commit-Position: refs/heads/master@{#29184}

Review URL: https://codereview.chromium.org/1189743003

Cr-Commit-Position: refs/heads/master@{#29192}

src/parser.h

@@ -538,6 +538,27 @@ class RegExpParser BASE_EMBEDDED {
 class Parser;
 class SingletonLogger;
 
+
+struct ParserFormalParameterParsingState
+    : public PreParserFormalParameterParsingState {
+  struct Parameter {
+    Parameter(Variable* var, Expression* pattern)
+        : var(var), pattern(pattern) {}
+    Variable* var;
+    Expression* pattern;
+  };
+
+  explicit ParserFormalParameterParsingState(Scope* scope)
+      : PreParserFormalParameterParsingState(scope), params(4, scope->zone()) {}
+
+  ZoneList<Parameter> params;
+
+  void AddParameter(Variable* var, Expression* pattern) {
+    params.Add(Parameter(var, pattern), scope->zone());
+  }
+};
+
+
 class ParserTraits {
  public:
   struct Type {
@@ -560,7 +581,7 @@ class ParserTraits {
     typedef ZoneList<v8::internal::Expression*>* ExpressionList;
     typedef ZoneList<ObjectLiteral::Property*>* PropertyList;
     typedef const v8::internal::AstRawString* FormalParameter;
-    typedef Scope FormalParameterScope;
+    typedef ParserFormalParameterParsingState FormalParameterParsingState;
     typedef ZoneList<v8::internal::Statement*>* StatementList;
 
     // For constructing objects returned by the traversing functions.
@@ -751,17 +772,21 @@ class ParserTraits {
   ZoneList<v8::internal::Statement*>* NewStatementList(int size, Zone* zone) {
     return new(zone) ZoneList<v8::internal::Statement*>(size, zone);
   }
+
+  V8_INLINE void AddParameterInitializationBlock(
+      const ParserFormalParameterParsingState& formal_parameters,
+      ZoneList<v8::internal::Statement*>* body, bool* ok);
+
   V8_INLINE Scope* NewScope(Scope* parent_scope, ScopeType scope_type,
                             FunctionKind kind = kNormalFunction);
 
-  V8_INLINE void DeclareFormalParameter(Scope* scope, Expression* name,
-                                        ExpressionClassifier* classifier,
-                                        bool is_rest);
-  void ParseArrowFunctionFormalParameters(Scope* scope, Expression* params,
-                                          const Scanner::Location& params_loc,
-                                          bool* has_rest,
-                                          Scanner::Location* duplicate_loc,
-                                          bool* ok);
+  V8_INLINE void DeclareFormalParameter(
+      ParserFormalParameterParsingState* parsing_state, Expression* name,
+      ExpressionClassifier* classifier, bool is_rest);
+  void ParseArrowFunctionFormalParameters(
+      ParserFormalParameterParsingState* scope, Expression* params,
+      const Scanner::Location& params_loc, Scanner::Location* duplicate_loc,
+      bool* ok);
 
   // Temporary glue; these functions will move to ParserBase.
   Expression* ParseV8Intrinsic(bool* ok);
@@ -774,8 +799,9 @@ class ParserTraits {
       int* materialized_literal_count, int* expected_property_count, bool* ok,
       Scanner::BookmarkScope* bookmark = nullptr);
   V8_INLINE ZoneList<Statement*>* ParseEagerFunctionBody(
-      const AstRawString* name, int pos, Variable* fvar,
-      Token::Value fvar_init_op, FunctionKind kind, bool* ok);
+      const AstRawString* name, int pos,
+      const ParserFormalParameterParsingState& formal_parameters,
+      Variable* fvar, Token::Value fvar_init_op, FunctionKind kind, bool* ok);
 
   ClassLiteral* ParseClassLiteral(const AstRawString* name,
                                   Scanner::Location class_name_location,
@@ -936,6 +962,7 @@ class Parser : public ParserBase<ParserTraits> {
                                 bool* ok);
 
   struct DeclarationDescriptor {
+    enum Kind { NORMAL, PARAMETER };
     Parser* parser;
     Scope* declaration_scope;
     Scope* scope;
@@ -945,6 +972,7 @@ class Parser : public ParserBase<ParserTraits> {
     int declaration_pos;
     int initialization_pos;
     Token::Value init_op;
+    Kind declaration_kind;
   };
 
   struct DeclarationParsingResult {
@@ -1095,7 +1123,9 @@ class Parser : public ParserBase<ParserTraits> {
 
   // Parser support
   VariableProxy* NewUnresolved(const AstRawString* name, VariableMode mode);
-  Variable* Declare(Declaration* declaration, bool resolve, bool* ok);
+  Variable* Declare(Declaration* declaration,
+                    DeclarationDescriptor::Kind declaration_kind, bool resolve,
+                    bool* ok);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label, bool* ok);
@@ -1121,10 +1151,14 @@ class Parser : public ParserBase<ParserTraits> {
   PreParser::PreParseResult ParseLazyFunctionBodyWithPreParser(
       SingletonLogger* logger, Scanner::BookmarkScope* bookmark = nullptr);
 
+  Block* BuildParameterInitializationBlock(
+      const ParserFormalParameterParsingState& formal_parameters, bool* ok);
+
   // Consumes the ending }.
   ZoneList<Statement*>* ParseEagerFunctionBody(
-      const AstRawString* function_name, int pos, Variable* fvar,
-      Token::Value fvar_init_op, FunctionKind kind, bool* ok);
+      const AstRawString* function_name, int pos,
+      const ParserFormalParameterParsingState& formal_parameters,
+      Variable* fvar, Token::Value fvar_init_op, FunctionKind kind, bool* ok);
 
   void ThrowPendingError(Isolate* isolate, Handle<Script> script);
 
@@ -1188,10 +1222,11 @@ void ParserTraits::SkipLazyFunctionBody(int* materialized_literal_count,
 
 
 ZoneList<Statement*>* ParserTraits::ParseEagerFunctionBody(
-    const AstRawString* name, int pos, Variable* fvar,
+    const AstRawString* name, int pos,
+    const ParserFormalParameterParsingState& formal_parameters, Variable* fvar,
     Token::Value fvar_init_op, FunctionKind kind, bool* ok) {
-  return parser_->ParseEagerFunctionBody(name, pos, fvar, fvar_init_op, kind,
-                                         ok);
+  return parser_->ParseEagerFunctionBody(name, pos, formal_parameters, fvar,
+                                         fvar_init_op, kind, ok);
 }
 
 void ParserTraits::CheckConflictingVarDeclarations(v8::internal::Scope* scope,
@@ -1270,18 +1305,20 @@ Expression* ParserTraits::SpreadCallNew(
 }
 
 
-void ParserTraits::DeclareFormalParameter(Scope* scope, Expression* pattern,
-                                          ExpressionClassifier* classifier,
-                                          bool is_rest) {
+void ParserTraits::DeclareFormalParameter(
+    ParserFormalParameterParsingState* parsing_state, Expression* pattern,
+    ExpressionClassifier* classifier, bool is_rest) {
   bool is_duplicate = false;
-  if (!pattern->IsVariableProxy()) {
-    // TODO(dslomov): implement.
-    DCHECK(parser_->allow_harmony_destructuring());
-    return;
-  }
-  auto name = pattern->AsVariableProxy()->raw_name();
-  Variable* var = scope->DeclareParameter(name, VAR, is_rest, &is_duplicate);
-  if (is_sloppy(scope->language_mode())) {
+  bool is_simple_name = pattern->IsVariableProxy();
+  DCHECK(parser_->allow_harmony_destructuring() || is_simple_name);
+
+  const AstRawString* name = is_simple_name
+                                 ? pattern->AsVariableProxy()->raw_name()
+                                 : parser_->ast_value_factory()->empty_string();
+  Variable* var =
+      parsing_state->scope->DeclareParameter(name, VAR, is_rest, &is_duplicate);
+  parsing_state->AddParameter(var, is_simple_name ? nullptr : pattern);
+  if (is_sloppy(parsing_state->scope->language_mode())) {
     // TODO(sigurds) Mark every parameter as maybe assigned. This is a
     // conservative approximation necessary to account for parameters
     // that are assigned via the arguments array.
@@ -1292,6 +1329,18 @@ void ParserTraits::DeclareFormalParameter(Scope* scope, Expression* pattern,
         parser_->scanner()->location());
   }
 }
+
+
+void ParserTraits::AddParameterInitializationBlock(
+    const ParserFormalParameterParsingState& formal_parameters,
+    ZoneList<v8::internal::Statement*>* body, bool* ok) {
+  auto* init_block =
+      parser_->BuildParameterInitializationBlock(formal_parameters, ok);
+  if (!*ok) return;
+  if (init_block != nullptr) {
+    body->Add(init_block, parser_->zone());
+  }
+}
 } }  // namespace v8::internal
 
 #endif  // V8_PARSER_H_

src/pattern-rewriter.cc

@@ -53,7 +53,8 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   Declaration* declaration = factory()->NewVariableDeclaration(
       proxy, descriptor_->mode, descriptor_->scope,
       descriptor_->declaration_pos);
-  Variable* var = parser->Declare(declaration, descriptor_->mode != VAR, ok_);
+  Variable* var = parser->Declare(declaration, descriptor_->declaration_kind,
+                                  descriptor_->mode != VAR, ok_);
   if (!*ok_) return;
   DCHECK_NOT_NULL(var);
   DCHECK(!proxy->is_resolved() || proxy->var() == var);

src/preparser.cc

@@ -1038,17 +1038,18 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   DuplicateFinder duplicate_finder(scanner()->unicode_cache());
   ExpressionClassifier formals_classifier(&duplicate_finder);
 
-  bool has_rest = false;
   Expect(Token::LPAREN, CHECK_OK);
   int start_position = scanner()->location().beg_pos;
   function_scope->set_start_position(start_position);
-  int num_parameters = ParseFormalParameterList(nullptr, &has_rest,
-                                                &formals_classifier, CHECK_OK);
+  PreParserFormalParameterParsingState parsing_state(nullptr);
+  int num_parameters =
+      ParseFormalParameterList(&parsing_state, &formals_classifier, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
   int formals_end_position = scanner()->location().end_pos;
 
-  CheckArityRestrictions(num_parameters, arity_restriction, has_rest,
-                         start_position, formals_end_position, CHECK_OK);
+  CheckArityRestrictions(num_parameters, arity_restriction,
+                         parsing_state.has_rest, start_position,
+                         formals_end_position, CHECK_OK);
 
   // See Parser::ParseFunctionLiteral for more information about lazy parsing
   // and lazy compilation.
@@ -1068,7 +1069,8 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   // function, since the function can declare itself strict.
   CheckFunctionName(language_mode(), kind, function_name,
                     name_is_strict_reserved, function_name_location, CHECK_OK);
-  const bool strict_formal_parameters = has_rest || IsConciseMethod(kind);
+  const bool strict_formal_parameters =
+      !parsing_state.is_simple_parameter_list || IsConciseMethod(kind);
   const bool allow_duplicate_parameters =
       is_sloppy(language_mode()) && !strict_formal_parameters;
   ValidateFormalParameters(&formals_classifier, language_mode(),

src/scopes.cc

@@ -466,15 +466,22 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode,
                                   bool is_rest, bool* is_duplicate) {
   DCHECK(!already_resolved());
   DCHECK(is_function_scope());
-  Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
-                                     kCreatedInitialized);
+
+  Variable* var;
+  if (!name->IsEmpty()) {
+    var = variables_.Declare(this, name, mode, Variable::NORMAL,
+                             kCreatedInitialized);
+    // TODO(wingo): Avoid O(n^2) check.
+    *is_duplicate = IsDeclaredParameter(name);
+  } else {
+    var = new (zone())
+        Variable(this, name, TEMPORARY, Variable::NORMAL, kCreatedInitialized);
+  }
   if (is_rest) {
     DCHECK_NULL(rest_parameter_);
     rest_parameter_ = var;
     rest_index_ = num_parameters();
   }
-  // TODO(wingo): Avoid O(n^2) check.
-  *is_duplicate = IsDeclaredParameter(name);
   params_.Add(var, zone());
   return var;
 }

src/snapshot/serialize.cc

@@ -2639,13 +2639,13 @@ SerializedCodeData::SerializedCodeData(const List<byte>& payload,
 SerializedCodeData::SanityCheckResult SerializedCodeData::SanityCheck(
     Isolate* isolate, String* source) const {
   uint32_t magic_number = GetMagicNumber();
+  if (magic_number != ComputeMagicNumber(isolate)) return MAGIC_NUMBER_MISMATCH;
   uint32_t version_hash = GetHeaderValue(kVersionHashOffset);
   uint32_t source_hash = GetHeaderValue(kSourceHashOffset);
   uint32_t cpu_features = GetHeaderValue(kCpuFeaturesOffset);
   uint32_t flags_hash = GetHeaderValue(kFlagHashOffset);
   uint32_t c1 = GetHeaderValue(kChecksum1Offset);
   uint32_t c2 = GetHeaderValue(kChecksum2Offset);
-  if (magic_number != ComputeMagicNumber(isolate)) return MAGIC_NUMBER_MISMATCH;
   if (version_hash != Version::Hash()) return VERSION_MISMATCH;
   if (source_hash != SourceHash(source)) return SOURCE_MISMATCH;
   if (cpu_features != static_cast<uint32_t>(CpuFeatures::SupportedFeatures())) {

test/cctest/test-parsing.cc

@@ -6389,7 +6389,6 @@ TEST(DestructuringPositiveTests) {
                                    {"function f(", ") {}"},
                                    {"function f(argument1, ", ") {}"},
                                    {"var f = (", ") => {};"},
-                                   {"var f = ", " => {};"},
                                    {"var f = (argument1,", ") => {};"},
                                    {NULL, NULL}};
 
@@ -6417,6 +6416,7 @@ TEST(DestructuringPositiveTests) {
     "{42 : x = 42}",
     "{42e-2 : x}",
     "{42e-2 : x = 42}",
+    "{x : y, x : z}",
     "{'hi' : x}",
     "{'hi' : x = 42}",
     "{var: x}",
@@ -6605,6 +6605,115 @@ TEST(DestructuringDisallowPatternsInForVarIn) {
 }
 
 
+TEST(DestructuringDuplicateParams) {
+  i::FLAG_harmony_destructuring = true;
+  i::FLAG_harmony_arrow_functions = true;
+  i::FLAG_harmony_computed_property_names = true;
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonyObjectLiterals, kAllowHarmonyComputedPropertyNames,
+      kAllowHarmonyArrowFunctions, kAllowHarmonyDestructuring};
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"function outer() { 'use strict';", "}"},
+                                   {nullptr, nullptr}};
+
+
+  // clang-format off
+  const char* error_data[] = {
+    "function f(x,x){}",
+    "function f(x, {x : x}){}",
+    "function f(x, {x}){}",
+    "function f({x,x}) {}",
+    "function f([x,x]) {}",
+    "function f(x, [y,{z:x}]) {}",
+    "function f([x,{y:x}]) {}",
+    // non-simple parameter list causes duplicates to be errors in sloppy mode.
+    "function f(x, x, {a}) {}",
+    nullptr};
+  // clang-format on
+  RunParserSyncTest(context_data, error_data, kError, NULL, 0, always_flags,
+                    arraysize(always_flags));
+}
+
+
+TEST(DestructuringDuplicateParamsSloppy) {
+  i::FLAG_harmony_destructuring = true;
+  i::FLAG_harmony_arrow_functions = true;
+  i::FLAG_harmony_computed_property_names = true;
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonyObjectLiterals, kAllowHarmonyComputedPropertyNames,
+      kAllowHarmonyArrowFunctions, kAllowHarmonyDestructuring};
+  const char* context_data[][2] = {
+      {"", ""}, {"function outer() {", "}"}, {nullptr, nullptr}};
+
+
+  // clang-format off
+  const char* error_data[] = {
+    // non-simple parameter list causes duplicates to be errors in sloppy mode.
+    "function f(x, {x : x}){}",
+    "function f(x, {x}){}",
+    "function f({x,x}) {}",
+    "function f(x, x, {a}) {}",
+    nullptr};
+  // clang-format on
+  RunParserSyncTest(context_data, error_data, kError, NULL, 0, always_flags,
+                    arraysize(always_flags));
+}
+
+
+TEST(DestructuringDisallowPatternsInSingleParamArrows) {
+  i::FLAG_harmony_destructuring = true;
+  i::FLAG_harmony_arrow_functions = true;
+  i::FLAG_harmony_computed_property_names = true;
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonyObjectLiterals, kAllowHarmonyComputedPropertyNames,
+      kAllowHarmonyArrowFunctions, kAllowHarmonyDestructuring};
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"function outer() { 'use strict';", "}"},
+                                   {"", ""},
+                                   {"function outer() { ", "}"},
+                                   {nullptr, nullptr}};
+
+  // clang-format off
+  const char* error_data[] = {
+    "var f = {x} => {};",
+    "var f = {x,y} => {};",
+    nullptr};
+  // clang-format on
+  RunParserSyncTest(context_data, error_data, kError, NULL, 0, always_flags,
+                    arraysize(always_flags));
+}
+
+
+TEST(DestructuringDisallowPatternsInRestParams) {
+  i::FLAG_harmony_destructuring = true;
+  i::FLAG_harmony_arrow_functions = true;
+  i::FLAG_harmony_rest_parameters = true;
+  i::FLAG_harmony_computed_property_names = true;
+  static const ParserFlag always_flags[] = {
+      kAllowHarmonyObjectLiterals, kAllowHarmonyComputedPropertyNames,
+      kAllowHarmonyArrowFunctions, kAllowHarmonyRestParameters,
+      kAllowHarmonyDestructuring};
+  const char* context_data[][2] = {{"'use strict';", ""},
+                                   {"function outer() { 'use strict';", "}"},
+                                   {"", ""},
+                                   {"function outer() { ", "}"},
+                                   {nullptr, nullptr}};
+
+  // clang-format off
+  const char* error_data[] = {
+    "function(...{}) {}",
+    "function(...{x}) {}",
+    "function(...[x]) {}",
+    "(...{}) => {}",
+    "(...{x}) => {}",
+    "(...[x]) => {}",
+    nullptr};
+  // clang-format on
+  RunParserSyncTest(context_data, error_data, kError, NULL, 0, always_flags,
+                    arraysize(always_flags));
+}
+
+
 TEST(SpreadArray) {
   i::FLAG_harmony_spread_arrays = true;
 

test/mjsunit/harmony/destructuring.js

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 //
 // Flags: --harmony-destructuring --harmony-computed-property-names
+// Flags: --harmony-arrow-functions
 
 (function TestObjectLiteralPattern() {
   var { x : x, y : y } = { x : 1, y : 2 };
@@ -686,3 +687,44 @@
   assertEquals('ab', sx);
   assertEquals('12', sy);
 }());
+
+
+(function TestParameters() {
+  function f({a, b}) { return a - b; }
+  assertEquals(1, f({a : 6, b : 5}));
+
+  function f1(c, {a, b}) { return c + a - b; }
+  assertEquals(8, f1(7, {a : 6, b : 5}));
+
+  function f2({c, d}, {a, b}) { return c - d + a - b; }
+  assertEquals(7, f2({c : 7, d : 1}, {a : 6, b : 5}));
+
+  function f3([{a, b}]) { return a - b; }
+  assertEquals(1, f3([{a : 6, b : 5}]));
+
+  var g = ({a, b}) => { return a - b; };
+  assertEquals(1, g({a : 6, b : 5}));
+
+  var g1 = (c, {a, b}) => { return c + a - b; };
+  assertEquals(8, g1(7, {a : 6, b : 5}));
+
+  var g2 = ({c, d}, {a, b}) => { return c - d + a - b; };
+  assertEquals(7, g2({c : 7, d : 1}, {a : 6, b : 5}));
+
+  var g3 = ([{a, b}]) => { return a - b; };
+  assertEquals(1, g3([{a : 6, b : 5}]));
+}());
+
+
+(function TestDuplicatesInParameters() {
+  assertThrows("'use strict';function f(x,x){}", SyntaxError);
+  assertThrows("'use strict';function f({x,x}){}", SyntaxError);
+  assertThrows("'use strict';function f(x, {x}){}", SyntaxError);
+  assertThrows("'use strict';var f = (x,x) => {};", SyntaxError);
+  assertThrows("'use strict';var f = ({x,x}) => {};", SyntaxError);
+  assertThrows("'use strict';var f = (x, {x}) => {};", SyntaxError);
+
+  function ok(x) { var x; }; ok();
+  assertThrows("function f({x}) { var x; }; f({});", SyntaxError);
+  assertThrows("'use strict'; function f({x}) { let x = 0; }; f({});", SyntaxError);
+}());

test/mjsunit/regress/regress-1130.js

@@ -35,6 +35,6 @@ try {
   eval("(function() { const x; var x })")();
 } catch (e) {
   exception = true;
-  assertTrue(e instanceof TypeError);
+  assertTrue(e instanceof SyntaxError);
 }
 assertTrue(exception);

test/mjsunit/regress/regress-436896.js

@@ -14,4 +14,4 @@ function g(x) {
 }
 
 %OptimizeFunctionOnNextCall(g);
-assertThrows(function() { g(42); }, TypeError);
+assertThrows(function() { g(42); }, SyntaxError);

test/webkit/fast/js/arguments-expected.txt

@@ -157,7 +157,7 @@ PASS access_after_delete_extra_5(1, 2, 3, 4, 5) is 5
 PASS argumentsParam(true) is true
 PASS argumentsFunctionConstructorParam(true) is true
 PASS argumentsVarUndefined() is '[object Arguments]'
-FAIL argumentsConstUndefined() should be [object Arguments]. Threw exception TypeError: Identifier 'arguments' has already been declared
+FAIL argumentsConstUndefined() should be [object Arguments]. Threw exception SyntaxError: Identifier 'arguments' has already been declared
 PASS argumentCalleeInException() is argumentCalleeInException
 PASS shadowedArgumentsApply([true]) is true
 PASS shadowedArgumentsLength([]) is 0