[destructuring] Adapting PatternRewriter to work in C-style for-statements.

BUG=v8:811
LOG=N

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

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

BUILD.gn

@@ -959,7 +959,6 @@ source_set("v8_base") {
     "src/ostreams.cc",
     "src/ostreams.h",
     "src/pattern-rewriter.cc",
-    "src/pattern-rewriter.h",
     "src/parser.cc",
     "src/parser.h",
     "src/pending-compilation-error-handler.cc",

src/ast.h

@@ -3352,6 +3352,7 @@ class AstNodeFactory final BASE_EMBEDDED {
                                   Variable::Kind variable_kind,
                                   int start_position = RelocInfo::kNoPosition,
                                   int end_position = RelocInfo::kNoPosition) {
+    DCHECK_NOT_NULL(name);
     return new (zone_)
         VariableProxy(zone_, name, variable_kind, start_position, end_position);
   }

src/parser.cc

@@ -14,7 +14,6 @@
 #include "src/compiler.h"
 #include "src/messages.h"
 #include "src/parser.h"
-#include "src/pattern-rewriter.h"
 #include "src/preparser.h"
 #include "src/runtime/runtime.h"
 #include "src/scanner-character-streams.h"
@@ -2272,30 +2271,58 @@ Block* Parser::ParseScopedBlock(ZoneList<const AstRawString*>* labels,
 }
 
 
+const AstRawString* Parser::DeclarationParsingResult::SingleName() const {
+  if (declarations.length() != 1) return nullptr;
+  const Declaration& declaration = declarations.at(0);
+  if (declaration.pattern->IsVariableProxy()) {
+    return declaration.pattern->AsVariableProxy()->raw_name();
+  }
+  return nullptr;
+}
+
+
+Block* Parser::DeclarationParsingResult::BuildInitializationBlock(
+    ZoneList<const AstRawString*>* names, bool* ok) {
+  Block* result =
+      descriptor.parser->factory()->NewBlock(NULL, 1, true, descriptor.pos);
+  for (auto declaration : declarations) {
+    PatternRewriter::DeclareAndInitializeVariables(
+        result, &descriptor, &declaration, names, CHECK_OK);
+  }
+  return result;
+}
+
+
 Block* Parser::ParseVariableStatement(VariableDeclarationContext var_context,
                                       ZoneList<const AstRawString*>* names,
                                       bool* ok) {
   // VariableStatement ::
   //   VariableDeclarations ';'
 
-  const AstRawString* ignore;
-  Block* result = ParseVariableDeclarations(
-      var_context, nullptr, names, &ignore, nullptr, nullptr, CHECK_OK);
+  // The scope of a var/const declared variable anywhere inside a function
+  // is the entire function (ECMA-262, 3rd, 10.1.3, and 12.2). Thus we can
+  // transform a source-level var/const declaration into a (Function)
+  // Scope declaration, and rewrite the source-level initialization into an
+  // assignment statement. We use a block to collect multiple assignments.
+  //
+  // We mark the block as initializer block because we don't want the
+  // rewriter to add a '.result' assignment to such a block (to get compliant
+  // behavior for code such as print(eval('var x = 7')), and for cosmetic
+  // reasons when pretty-printing. Also, unless an assignment (initialization)
+  // is inside an initializer block, it is ignored.
+
+  DeclarationParsingResult parsing_result;
+  ParseVariableDeclarations(var_context, &parsing_result, CHECK_OK);
   ExpectSemicolon(CHECK_OK);
+
+  Block* result = parsing_result.BuildInitializationBlock(names, CHECK_OK);
   return result;
 }
 
 
-// If the variable declaration declares exactly one non-const
-// variable, then *out is set to that variable. In all other cases,
-// *out is untouched; in particular, it is the caller's responsibility
-// to initialize it properly. This mechanism is used for the parsing
-// of 'for-in' loops.
-Block* Parser::ParseVariableDeclarations(
-    VariableDeclarationContext var_context, int* num_decl,
-    ZoneList<const AstRawString*>* names, const AstRawString** out,
-    Scanner::Location* first_initializer_loc, Scanner::Location* bindings_loc,
-    bool* ok) {
+void Parser::ParseVariableDeclarations(VariableDeclarationContext var_context,
+                                       DeclarationParsingResult* parsing_result,
+                                       bool* ok) {
   // VariableDeclarations ::
   //   ('var' | 'const' | 'let') (Identifier ('=' AssignmentExpression)?)+[',']
   //
@@ -2310,125 +2337,108 @@ Block* Parser::ParseVariableDeclarations(
   // ConstBinding ::
   //   BindingPattern '=' AssignmentExpression
 
-  PatternRewriter::DeclarationDescriptor decl;
-  decl.parser = this;
-  decl.pos = peek_position();
-  decl.mode = VAR;
+  parsing_result->descriptor.parser = this;
+  parsing_result->descriptor.pos = peek_position();
+  parsing_result->descriptor.mode = VAR;
   // True if the binding needs initialization. 'let' and 'const' declared
   // bindings are created uninitialized by their declaration nodes and
   // need initialization. 'var' declared bindings are always initialized
   // immediately by their declaration nodes.
-  decl.needs_init = false;
-  decl.is_const = false;
-  decl.init_op = Token::INIT_VAR;
-  decl.names = names;
+  parsing_result->descriptor.needs_init = false;
+  parsing_result->descriptor.is_const = false;
+  parsing_result->descriptor.init_op = Token::INIT_VAR;
   if (peek() == Token::VAR) {
     if (is_strong(language_mode())) {
       Scanner::Location location = scanner()->peek_location();
       ReportMessageAt(location, "strong_var");
       *ok = false;
-      return NULL;
+      return;
     }
     Consume(Token::VAR);
   } else if (peek() == Token::CONST) {
     Consume(Token::CONST);
     if (is_sloppy(language_mode())) {
-      decl.mode = CONST_LEGACY;
-      decl.init_op = Token::INIT_CONST_LEGACY;
+      parsing_result->descriptor.mode = CONST_LEGACY;
+      parsing_result->descriptor.init_op = Token::INIT_CONST_LEGACY;
       ++use_counts_[v8::Isolate::kLegacyConst];
     } else {
       DCHECK(var_context != kStatement);
-      decl.mode = CONST;
-      decl.init_op = Token::INIT_CONST;
+      parsing_result->descriptor.mode = CONST;
+      parsing_result->descriptor.init_op = Token::INIT_CONST;
     }
-    decl.is_const = true;
-    decl.needs_init = true;
+    parsing_result->descriptor.is_const = true;
+    parsing_result->descriptor.needs_init = true;
   } else if (peek() == Token::LET && is_strict(language_mode())) {
     Consume(Token::LET);
     DCHECK(var_context != kStatement);
-    decl.mode = LET;
-    decl.needs_init = true;
-    decl.init_op = Token::INIT_LET;
+    parsing_result->descriptor.mode = LET;
+    parsing_result->descriptor.needs_init = true;
+    parsing_result->descriptor.init_op = Token::INIT_LET;
   } else {
     UNREACHABLE();  // by current callers
   }
 
-  decl.declaration_scope = DeclarationScope(decl.mode);
-  decl.scope = scope_;
+  parsing_result->descriptor.declaration_scope =
+      DeclarationScope(parsing_result->descriptor.mode);
+  parsing_result->descriptor.scope = scope_;
 
 
-  // The scope of a var/const declared variable anywhere inside a function
-  // is the entire function (ECMA-262, 3rd, 10.1.3, and 12.2). Thus we can
-  // transform a source-level var/const declaration into a (Function)
-  // Scope declaration, and rewrite the source-level initialization into an
-  // assignment statement. We use a block to collect multiple assignments.
-  //
-  // We mark the block as initializer block because we don't want the
-  // rewriter to add a '.result' assignment to such a block (to get compliant
-  // behavior for code such as print(eval('var x = 7')), and for cosmetic
-  // reasons when pretty-printing. Also, unless an assignment (initialization)
-  // is inside an initializer block, it is ignored.
-  //
-  // Create new block with one expected declaration.
-  decl.block = factory()->NewBlock(NULL, 1, true, decl.pos);
-  int nvars = 0;  // the number of variables declared
+  bool first_declaration = true;
   int bindings_start = peek_position();
-  const AstRawString* first_name = NULL;
   bool is_for_iteration_variable;
   do {
     if (fni_ != NULL) fni_->Enter();
 
-    // Parse variable name.
-    if (nvars > 0) Consume(Token::COMMA);
+    // Parse name.
+    if (!first_declaration) Consume(Token::COMMA);
 
-    PatternRewriter pattern_rewriter;
+    Expression* pattern;
     {
       ExpressionClassifier pattern_classifier;
       Token::Value next = peek();
-      Expression* pattern =
-          ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
-      ValidateBindingPattern(&pattern_classifier, CHECK_OK);
-      pattern_rewriter = PatternRewriter(&decl, pattern);
-      if (!allow_harmony_destructuring() &&
-          !pattern_rewriter.IsSingleVariableBinding()) {
+      pattern = ParsePrimaryExpression(&pattern_classifier, ok);
+      if (!*ok) return;
+      ValidateBindingPattern(&pattern_classifier, ok);
+      if (!*ok) return;
+      if (!allow_harmony_destructuring() && !pattern->IsVariableProxy()) {
         ReportUnexpectedToken(next);
         *ok = false;
-        return nullptr;
+        return;
       }
-
-      // TODO(dslomov): unify
     }
 
     Scanner::Location variable_loc = scanner()->location();
-    const bool single_name = pattern_rewriter.IsSingleVariableBinding();
-    if (single_name) {
-      if (!first_name) first_name = pattern_rewriter.SingleName();
-      if (fni_ != NULL) fni_->PushVariableName(pattern_rewriter.SingleName());
+    const AstRawString* single_name =
+        pattern->IsVariableProxy() ? pattern->AsVariableProxy()->raw_name()
+                                   : nullptr;
+    if (single_name != nullptr) {
+      if (fni_ != NULL) fni_->PushVariableName(single_name);
     }
 
     is_for_iteration_variable =
         var_context == kForStatement &&
         (peek() == Token::IN || PeekContextualKeyword(CStrVector("of")));
-    if (is_for_iteration_variable && decl.mode == CONST) {
-      decl.needs_init = false;
+    if (is_for_iteration_variable && parsing_result->descriptor.mode == CONST) {
+      parsing_result->descriptor.needs_init = false;
     }
 
     Expression* value = NULL;
-    decl.pos = -1;
-    decl.initializer_position = -1;
     // Harmony consts have non-optional initializers.
-    if (peek() == Token::ASSIGN ||
-        (decl.mode == CONST && !is_for_iteration_variable)) {
-      Expect(Token::ASSIGN, CHECK_OK);
-      decl.pos = position();
+    int initializer_position = RelocInfo::kNoPosition;
+    if (peek() == Token::ASSIGN || (parsing_result->descriptor.mode == CONST &&
+                                    !is_for_iteration_variable)) {
+      Expect(Token::ASSIGN, ok);
+      if (!*ok) return;
       ExpressionClassifier classifier;
       value = ParseAssignmentExpression(var_context != kForStatement,
-                                        &classifier, CHECK_OK);
-      ValidateExpression(&classifier, CHECK_OK);
+                                        &classifier, ok);
+      if (!*ok) return;
+      ValidateExpression(&classifier, ok);
+      if (!*ok) return;
       variable_loc.end_pos = scanner()->location().end_pos;
 
-      if (first_initializer_loc && !first_initializer_loc->IsValid()) {
-        *first_initializer_loc = variable_loc;
+      if (!parsing_result->first_initializer_loc.IsValid()) {
+        parsing_result->first_initializer_loc = variable_loc;
       }
 
       // Don't infer if it is "a = function(){...}();"-like expression.
@@ -2441,31 +2451,25 @@ Block* Parser::ParseVariableDeclarations(
         }
       }
       // End position of the initializer is after the assignment expression.
-      decl.initializer_position = scanner()->location().end_pos;
+      initializer_position = scanner()->location().end_pos;
     } else {
       // End position of the initializer is after the variable.
-      decl.initializer_position = position();
+      initializer_position = position();
     }
 
     // Make sure that 'const x' and 'let x' initialize 'x' to undefined.
-    if (value == NULL && decl.needs_init) {
+    if (value == NULL && parsing_result->descriptor.needs_init) {
       value = GetLiteralUndefined(position());
     }
 
-    pattern_rewriter.DeclareAndInitializeVariables(value, &nvars, CHECK_OK);
-
     if (single_name && fni_ != NULL) fni_->Leave();
+    parsing_result->declarations.Add(DeclarationParsingResult::Declaration(
+        pattern, initializer_position, value));
+    first_declaration = false;
   } while (peek() == Token::COMMA);
 
-  if (bindings_loc) {
-    *bindings_loc =
-        Scanner::Location(bindings_start, scanner()->location().end_pos);
-  }
-
-  if (num_decl) *num_decl = nvars;
-  *out = first_name;
-
-  return decl.block;
+  parsing_result->bindings_loc =
+      Scanner::Location(bindings_start, scanner()->location().end_pos);
 }
 
 
@@ -3232,7 +3236,6 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   Block* inner_block = factory()->NewBlock(NULL, names->length() + 4, false,
                                            RelocInfo::kNoPosition);
   ZoneList<Variable*> inner_vars(names->length(), zone());
-
   // For each let variable x:
   //    make statement: let/const x = temp_x.
   VariableMode mode = is_const ? CONST : LET;
@@ -3248,6 +3251,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
                                  proxy, temp_proxy, RelocInfo::kNoPosition);
     Statement* assignment_statement =
         factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
+    DCHECK(init->position() != RelocInfo::kNoPosition);
     proxy->var()->set_initializer_position(init->position());
     inner_block->AddStatement(assignment_statement, zone());
   }
@@ -3384,21 +3388,19 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
   Scope* saved_scope = scope_;
   Scope* for_scope = NewScope(scope_, BLOCK_SCOPE);
   scope_ = for_scope;
-
   Expect(Token::FOR, CHECK_OK);
   Expect(Token::LPAREN, CHECK_OK);
   for_scope->set_start_position(scanner()->location().beg_pos);
   bool is_let_identifier_expression = false;
+  DeclarationParsingResult parsing_result;
   if (peek() != Token::SEMICOLON) {
     if (peek() == Token::VAR ||
         (peek() == Token::CONST && is_sloppy(language_mode()))) {
-      const AstRawString* name = NULL;
-      Scanner::Location first_initializer_loc = Scanner::Location::invalid();
-      Scanner::Location bindings_loc = Scanner::Location::invalid();
-      int num_decl;
-      Block* variable_statement = ParseVariableDeclarations(
-          kForStatement, &num_decl, nullptr, &name, &first_initializer_loc,
-          &bindings_loc, CHECK_OK);
+      ParseVariableDeclarations(kForStatement, &parsing_result, CHECK_OK);
+      Block* variable_statement =
+          parsing_result.BuildInitializationBlock(nullptr, CHECK_OK);
+
+      int num_decl = parsing_result.declarations.length();
       bool accept_IN = num_decl >= 1;
       bool accept_OF = true;
       ForEachStatement::VisitMode mode;
@@ -3410,18 +3412,21 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         if (num_decl != 1) {
           const char* loop_type =
               mode == ForEachStatement::ITERATE ? "for-of" : "for-in";
-          ParserTraits::ReportMessageAt(
-              bindings_loc, "for_inof_loop_multi_bindings", loop_type);
+          ParserTraits::ReportMessageAt(parsing_result.bindings_loc,
+                                        "for_inof_loop_multi_bindings",
+                                        loop_type);
           *ok = false;
           return nullptr;
         }
-        if (first_initializer_loc.IsValid() &&
+        if (parsing_result.first_initializer_loc.IsValid() &&
             (is_strict(language_mode()) || mode == ForEachStatement::ITERATE)) {
           if (mode == ForEachStatement::ITERATE) {
-            ReportMessageAt(first_initializer_loc, "for_of_loop_initializer");
+            ReportMessageAt(parsing_result.first_initializer_loc,
+                            "for_of_loop_initializer");
           } else {
             // TODO(caitp): This should be an error in sloppy mode too.
-            ReportMessageAt(first_initializer_loc, "for_in_loop_initializer");
+            ReportMessageAt(parsing_result.first_initializer_loc,
+                            "for_in_loop_initializer");
           }
           *ok = false;
           return nullptr;
@@ -3433,8 +3438,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         Expression* enumerable = ParseExpression(true, CHECK_OK);
         Expect(Token::RPAREN, CHECK_OK);
 
-        VariableProxy* each =
-            scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos);
+        VariableProxy* each = scope_->NewUnresolved(
+            factory(), parsing_result.SingleName(), each_beg_pos, each_end_pos);
         Statement* body = ParseSubStatement(NULL, CHECK_OK);
         InitializeForEachStatement(loop, each, enumerable, body);
         Block* result =
@@ -3453,13 +3458,12 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
     } else if ((peek() == Token::LET || peek() == Token::CONST) &&
                is_strict(language_mode())) {
       is_const = peek() == Token::CONST;
-      const AstRawString* name = NULL;
-      Scanner::Location first_initializer_loc = Scanner::Location::invalid();
-      Scanner::Location bindings_loc = Scanner::Location::invalid();
-      int num_decl;
-      Block* variable_statement = ParseVariableDeclarations(
-          kForStatement, &num_decl, &lexical_bindings, &name,
-          &first_initializer_loc, &bindings_loc, CHECK_OK);
+      ParseVariableDeclarations(kForStatement, &parsing_result, CHECK_OK);
+      DCHECK(parsing_result.descriptor.pos != RelocInfo::kNoPosition);
+      Block* variable_statement =
+          parsing_result.BuildInitializationBlock(&lexical_bindings, CHECK_OK);
+
+      int num_decl = parsing_result.declarations.length();
       bool accept_IN = num_decl >= 1;
       bool accept_OF = true;
       ForEachStatement::VisitMode mode;
@@ -3471,17 +3475,20 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         if (num_decl != 1) {
           const char* loop_type =
               mode == ForEachStatement::ITERATE ? "for-of" : "for-in";
-          ParserTraits::ReportMessageAt(
-              bindings_loc, "for_inof_loop_multi_bindings", loop_type);
+          ParserTraits::ReportMessageAt(parsing_result.bindings_loc,
+                                        "for_inof_loop_multi_bindings",
+                                        loop_type);
           *ok = false;
           return nullptr;
         }
-        if (first_initializer_loc.IsValid() &&
+        if (parsing_result.first_initializer_loc.IsValid() &&
             (is_strict(language_mode()) || mode == ForEachStatement::ITERATE)) {
           if (mode == ForEachStatement::ITERATE) {
-            ReportMessageAt(first_initializer_loc, "for_of_loop_initializer");
+            ReportMessageAt(parsing_result.first_initializer_loc,
+                            "for_of_loop_initializer");
           } else {
-            ReportMessageAt(first_initializer_loc, "for_in_loop_initializer");
+            ReportMessageAt(parsing_result.first_initializer_loc,
+                            "for_in_loop_initializer");
           }
           *ok = false;
           return nullptr;
@@ -3515,8 +3522,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         scope_ = for_scope;
         Expect(Token::RPAREN, CHECK_OK);
 
-        VariableProxy* each =
-            scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos);
+        VariableProxy* each = scope_->NewUnresolved(
+            factory(), parsing_result.SingleName(), each_end_pos);
         Statement* body = ParseSubStatement(NULL, CHECK_OK);
         Block* body_block =
             factory()->NewBlock(NULL, 3, false, RelocInfo::kNoPosition);

src/parser.h

@@ -871,7 +871,6 @@ class Parser : public ParserBase<ParserTraits> {
 
  private:
   friend class ParserTraits;
-  class PatternRewriter;
 
   // Limit the allowed number of local variables in a function. The hard limit
   // is that offsets computed by FullCodeGenerator::StackOperand and similar
@@ -940,12 +939,91 @@ class Parser : public ParserBase<ParserTraits> {
   Block* ParseVariableStatement(VariableDeclarationContext var_context,
                                 ZoneList<const AstRawString*>* names,
                                 bool* ok);
-  Block* ParseVariableDeclarations(VariableDeclarationContext var_context,
-                                   int* num_decl,
-                                   ZoneList<const AstRawString*>* names,
-                                   const AstRawString** out,
-                                   Scanner::Location* first_initializer_loc,
-                                   Scanner::Location* bindings_loc, bool* ok);
+
+  struct DeclarationDescriptor {
+    Parser* parser;
+    Scope* declaration_scope;
+    Scope* scope;
+    VariableMode mode;
+    bool is_const;
+    bool needs_init;
+    int pos;
+    Token::Value init_op;
+  };
+
+  struct DeclarationParsingResult {
+    struct Declaration {
+      Declaration(Expression* pattern, int initializer_position,
+                  Expression* initializer)
+          : pattern(pattern),
+            initializer_position(initializer_position),
+            initializer(initializer) {}
+
+      Expression* pattern;
+      int initializer_position;
+      Expression* initializer;
+    };
+
+    DeclarationParsingResult()
+        : declarations(4),
+          first_initializer_loc(Scanner::Location::invalid()),
+          bindings_loc(Scanner::Location::invalid()) {}
+
+    Block* BuildInitializationBlock(ZoneList<const AstRawString*>* names,
+                                    bool* ok);
+    const AstRawString* SingleName() const;
+
+    DeclarationDescriptor descriptor;
+    List<Declaration> declarations;
+    Scanner::Location first_initializer_loc;
+    Scanner::Location bindings_loc;
+  };
+
+  class PatternRewriter : private AstVisitor {
+   public:
+    static void DeclareAndInitializeVariables(
+        Block* block, const DeclarationDescriptor* declaration_descriptor,
+        const DeclarationParsingResult::Declaration* declaration,
+        ZoneList<const AstRawString*>* names, bool* ok);
+
+    void set_initializer_position(int pos) { initializer_position_ = pos; }
+
+   private:
+    PatternRewriter() {}
+
+#define DECLARE_VISIT(type) void Visit##type(v8::internal::type* node) override;
+    // Visiting functions for AST nodes make this an AstVisitor.
+    AST_NODE_LIST(DECLARE_VISIT)
+#undef DECLARE_VISIT
+    virtual void Visit(AstNode* node) override;
+
+    void RecurseIntoSubpattern(AstNode* pattern, Expression* value) {
+      Expression* old_value = current_value_;
+      current_value_ = value;
+      pattern->Accept(this);
+      current_value_ = old_value;
+    }
+
+    AstNodeFactory* factory() const { return descriptor_->parser->factory(); }
+    AstValueFactory* ast_value_factory() const {
+      return descriptor_->parser->ast_value_factory();
+    }
+    bool inside_with() const { return descriptor_->parser->inside_with(); }
+    Zone* zone() const { return descriptor_->parser->zone(); }
+
+    Expression* pattern_;
+    int initializer_position_;
+    Block* block_;
+    const DeclarationDescriptor* descriptor_;
+    ZoneList<const AstRawString*>* names_;
+    Expression* current_value_;
+    bool* ok_;
+  };
+
+
+  void ParseVariableDeclarations(VariableDeclarationContext var_context,
+                                 DeclarationParsingResult* parsing_result,
+                                 bool* ok);
   Statement* ParseExpressionOrLabelledStatement(
       ZoneList<const AstRawString*>* labels, bool* ok);
   IfStatement* ParseIfStatement(ZoneList<const AstRawString*>* labels,

src/pattern-rewriter.cc

@@ -4,38 +4,32 @@
 
 #include "src/ast.h"
 #include "src/parser.h"
-#include "src/pattern-rewriter.h"
 
 namespace v8 {
 
 namespace internal {
 
 
-bool Parser::PatternRewriter::IsSingleVariableBinding() const {
-  return pattern_->IsVariableProxy();
-}
-
-
-const AstRawString* Parser::PatternRewriter::SingleName() const {
-  DCHECK(IsSingleVariableBinding());
-  return pattern_->AsVariableProxy()->raw_name();
-}
+void Parser::PatternRewriter::DeclareAndInitializeVariables(
+    Block* block, const DeclarationDescriptor* declaration_descriptor,
+    const DeclarationParsingResult::Declaration* declaration,
+    ZoneList<const AstRawString*>* names, bool* ok) {
+  PatternRewriter rewriter;
 
+  rewriter.pattern_ = declaration->pattern;
+  rewriter.initializer_position_ = declaration->initializer_position;
+  rewriter.block_ = block;
+  rewriter.descriptor_ = declaration_descriptor;
+  rewriter.names_ = names;
+  rewriter.ok_ = ok;
 
-void Parser::PatternRewriter::DeclareAndInitializeVariables(Expression* value,
-                                                            int* nvars,
-                                                            bool* ok) {
-  ok_ = ok;
-  nvars_ = nvars;
-  RecurseIntoSubpattern(pattern_, value);
-  ok_ = nullptr;
-  nvars_ = nullptr;
+  rewriter.RecurseIntoSubpattern(rewriter.pattern_, declaration->initializer);
 }
 
 
 void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   Expression* value = current_value_;
-  decl_->scope->RemoveUnresolved(pattern->AsVariableProxy());
+  descriptor_->scope->RemoveUnresolved(pattern->AsVariableProxy());
 
   // Declare variable.
   // Note that we *always* must treat the initial value via a separate init
@@ -52,24 +46,27 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // For let/const declarations in harmony mode, we can also immediately
   // pre-resolve the proxy because it resides in the same scope as the
   // declaration.
-  Parser* parser = decl_->parser;
+  Parser* parser = descriptor_->parser;
   const AstRawString* name = pattern->raw_name();
-  VariableProxy* proxy = parser->NewUnresolved(name, decl_->mode);
+  VariableProxy* proxy = parser->NewUnresolved(name, descriptor_->mode);
   Declaration* declaration = factory()->NewVariableDeclaration(
-      proxy, decl_->mode, decl_->scope, decl_->pos);
-  Variable* var = parser->Declare(declaration, decl_->mode != VAR, ok_);
+      proxy, descriptor_->mode, descriptor_->scope, descriptor_->pos);
+  Variable* var = parser->Declare(declaration, descriptor_->mode != VAR, ok_);
   if (!*ok_) return;
   DCHECK_NOT_NULL(var);
   DCHECK(!proxy->is_resolved() || proxy->var() == var);
-  var->set_initializer_position(decl_->initializer_position);
-  (*nvars_)++;
-  if (decl_->declaration_scope->num_var_or_const() > kMaxNumFunctionLocals) {
+  var->set_initializer_position(initializer_position_);
+
+  DCHECK(initializer_position_ != RelocInfo::kNoPosition);
+
+  if (descriptor_->declaration_scope->num_var_or_const() >
+      kMaxNumFunctionLocals) {
     parser->ReportMessage("too_many_variables");
     *ok_ = false;
     return;
   }
-  if (decl_->names) {
-    decl_->names->Add(name, zone());
+  if (names_) {
+    names_->Add(name, zone());
   }
 
   // Initialize variables if needed. A
@@ -98,8 +95,9 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // The "variable" c initialized to x is the same as the declared
   // one - there is no re-lookup (see the last parameter of the
   // Declare() call above).
-  Scope* initialization_scope =
-      decl_->is_const ? decl_->declaration_scope : decl_->scope;
+  Scope* initialization_scope = descriptor_->is_const
+                                    ? descriptor_->declaration_scope
+                                    : descriptor_->scope;
 
 
   // Global variable declarations must be compiled in a specific
@@ -121,16 +119,16 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // browsers where the global object (window) has lots of
   // properties defined in prototype objects.
   if (initialization_scope->is_script_scope() &&
-      !IsLexicalVariableMode(decl_->mode)) {
+      !IsLexicalVariableMode(descriptor_->mode)) {
     // Compute the arguments for the runtime
     // call.test-parsing/InitializedDeclarationsInStrictForOfError
     ZoneList<Expression*>* arguments =
         new (zone()) ZoneList<Expression*>(3, zone());
     // We have at least 1 parameter.
-    arguments->Add(factory()->NewStringLiteral(name, decl_->pos), zone());
+    arguments->Add(factory()->NewStringLiteral(name, descriptor_->pos), zone());
     CallRuntime* initialize;
 
-    if (decl_->is_const) {
+    if (descriptor_->is_const) {
       arguments->Add(value, zone());
       value = NULL;  // zap the value to avoid the unnecessary assignment
 
@@ -141,13 +139,13 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
       initialize = factory()->NewCallRuntime(
           ast_value_factory()->initialize_const_global_string(),
           Runtime::FunctionForId(Runtime::kInitializeConstGlobal), arguments,
-          decl_->pos);
+          descriptor_->pos);
     } else {
       // Add language mode.
       // We may want to pass singleton to avoid Literal allocations.
       LanguageMode language_mode = initialization_scope->language_mode();
-      arguments->Add(factory()->NewNumberLiteral(language_mode, decl_->pos),
-                     zone());
+      arguments->Add(
+          factory()->NewNumberLiteral(language_mode, descriptor_->pos), zone());
 
       // Be careful not to assign a value to the global variable if
       // we're in a with. The initialization value should not
@@ -161,18 +159,19 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
         initialize = factory()->NewCallRuntime(
             ast_value_factory()->initialize_var_global_string(),
             Runtime::FunctionForId(Runtime::kInitializeVarGlobal), arguments,
-            decl_->pos);
+            descriptor_->pos);
       } else {
         initialize = NULL;
       }
     }
 
     if (initialize != NULL) {
-      decl_->block->AddStatement(
+      block_->AddStatement(
           factory()->NewExpressionStatement(initialize, RelocInfo::kNoPosition),
           zone());
     }
-  } else if (decl_->needs_init) {
+  } else if (value != nullptr && (descriptor_->needs_init ||
+                                  IsLexicalVariableMode(descriptor_->mode))) {
     // Constant initializations always assign to the declared constant which
     // is always at the function scope level. This is only relevant for
     // dynamically looked-up variables and constants (the
@@ -183,9 +182,9 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
     DCHECK_NOT_NULL(proxy);
     DCHECK_NOT_NULL(proxy->var());
     DCHECK_NOT_NULL(value);
-    Assignment* assignment =
-        factory()->NewAssignment(decl_->init_op, proxy, value, decl_->pos);
-    decl_->block->AddStatement(
+    Assignment* assignment = factory()->NewAssignment(
+        descriptor_->init_op, proxy, value, descriptor_->pos);
+    block_->AddStatement(
         factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition),
         zone());
     value = NULL;
@@ -194,14 +193,14 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // Add an assignment node to the initialization statement block if we still
   // have a pending initialization value.
   if (value != NULL) {
-    DCHECK(decl_->mode == VAR);
+    DCHECK(descriptor_->mode == VAR);
     // 'var' initializations are simply assignments (with all the consequences
     // if they are inside a 'with' statement - they may change a 'with' object
     // property).
     VariableProxy* proxy = initialization_scope->NewUnresolved(factory(), name);
-    Assignment* assignment =
-        factory()->NewAssignment(decl_->init_op, proxy, value, decl_->pos);
-    decl_->block->AddStatement(
+    Assignment* assignment = factory()->NewAssignment(
+        descriptor_->init_op, proxy, value, descriptor_->pos);
+    block_->AddStatement(
         factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition),
         zone());
   }
@@ -209,12 +208,12 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
 
 
 void Parser::PatternRewriter::VisitObjectLiteral(ObjectLiteral* pattern) {
-  auto temp = decl_->declaration_scope->NewTemporary(
+  auto temp = descriptor_->declaration_scope->NewTemporary(
       ast_value_factory()->empty_string());
   auto assignment =
       factory()->NewAssignment(Token::ASSIGN, factory()->NewVariableProxy(temp),
                                current_value_, RelocInfo::kNoPosition);
-  decl_->block->AddStatement(
+  block_->AddStatement(
       factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition),
       zone());
   for (ObjectLiteralProperty* property : *pattern->properties()) {

src/pattern-rewriter.h

-// Copyright 2015 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef V8_PATTERN_MATCHER_H_
-#define V8_PATTERN_MATCHER_H_
-
-#include "src/ast.h"
-#include "src/parser.h"
-
-namespace v8 {
-
-namespace internal {
-
-class Parser::PatternRewriter : private AstVisitor {
- public:
-  struct DeclarationDescriptor {
-    Parser* parser;
-    Scope* declaration_scope;
-    Scope* scope;
-    int initializer_position;
-    VariableMode mode;
-    ZoneList<const AstRawString*>* names;
-    bool is_const;
-    Block* block;
-    bool needs_init;
-    int pos;
-    Token::Value init_op;
-  };
-
-  explicit PatternRewriter(const DeclarationDescriptor* decl,
-                           Expression* pattern)
-      : decl_(decl),
-        pattern_(pattern),
-        current_value_(nullptr),
-        ok_(nullptr),
-        nvars_(nullptr) {}
-
-  PatternRewriter()
-      : decl_(nullptr),
-        pattern_(nullptr),
-        current_value_(nullptr),
-        ok_(nullptr),
-        nvars_(nullptr) {}
-
-  bool IsSingleVariableBinding() const;
-  const AstRawString* SingleName() const;
-
-  void DeclareAndInitializeVariables(Expression* value, int* nvars, bool* ok);
-
- private:
-#define DECLARE_VISIT(type) void Visit##type(v8::internal::type* node) override;
-  // Visiting functions for AST nodes make this an AstVisitor.
-  AST_NODE_LIST(DECLARE_VISIT)
-#undef DECLARE_VISIT
-  virtual void Visit(AstNode* node) override;
-
-  void RecurseIntoSubpattern(AstNode* pattern, Expression* value) {
-    Expression* old_value = current_value_;
-    current_value_ = value;
-    pattern->Accept(this);
-    current_value_ = old_value;
-  }
-
-  AstNodeFactory* factory() const { return decl_->parser->factory(); }
-  AstValueFactory* ast_value_factory() const {
-    return decl_->parser->ast_value_factory();
-  }
-  bool inside_with() const { return decl_->parser->inside_with(); }
-  Zone* zone() const { return decl_->parser->zone(); }
-
-  const DeclarationDescriptor* decl_;
-  Expression* pattern_;
-  Expression* current_value_;
-  bool* ok_;
-  int* nvars_;
-};
-}
-}  // namespace v8::internal
-
-
-#endif  // V8_PATTERN_MATCHER_H_

test/mjsunit/harmony/destructuring.js

@@ -11,4 +11,46 @@
 
   var {z} = { z : 3 };
   assertEquals(3, z);
+
+
+  var sum = 0;
+  for(var {z} = { z : 3 }; z != 0; z--) {
+    sum += z;
+  }
+  assertEquals(6, sum);
+}());
+
+
+(function TestObjectLiteralPatternLexical() {
+  'use strict';
+  let { x : x, y : y } = { x : 1, y : 2 };
+  assertEquals(1, x);
+  assertEquals(2, y);
+
+  let {z} = { z : 3 };
+  assertEquals(3, z);
+
+
+  let sum = 0;
+  for(let {x, z} = { x : 0, z : 3 }; z != 0; z--) {
+    assertEquals(0, x);
+    sum += z;
+  }
+  assertEquals(6, sum);
+}());
+
+
+(function TestObjectLiteralPatternLexicalConst() {
+  'use strict';
+  const { x : x, y : y } = { x : 1, y : 2 };
+  assertEquals(1, x);
+  assertEquals(2, y);
+
+  const {z} = { z : 3 };
+  assertEquals(3, z);
+
+
+  for(const {x, z} = { x : 0, z : 3 }; z != 3 || x != 0;) {
+    assertTrue(false);
+  }
 }());

tools/gyp/v8.gyp

@@ -799,7 +799,6 @@
         '../../src/ostreams.cc',
         '../../src/ostreams.h',
         '../../src/pattern-rewriter.cc',
-        '../../src/pattern-rewriter.h',
         '../../src/parser.cc',
         '../../src/parser.h',
         '../../src/pending-compilation-error-handler.cc',