Disallow destructuring in legacy sloppy for-in loop parsing

For web compat reasons, we support an initializer in the declaration
part of a for-in loop. But we should disallow this for destructured
declarations (just as we do for lexical declarations). In fact, without
disallowing it, we crash.

Also fix up the PreParser to have the same restrictions here as the parser
(the lexical check was missing there), verified by running the message tests
with --min-preparse-length=0.

In fixing the logic I've also cleaned up the code a bit, removing the
only-called-once DeclarationParsingResult::SingleName method.

BUG=v8:811
LOG=n

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

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

src/parser.cc

@@ -2381,16 +2381,6 @@ Block* Parser::ParseBlock(ZoneList<const AstRawString*>* labels, bool* ok) {
 }
 
 
-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(
@@ -3678,9 +3668,12 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
           *ok = false;
           return nullptr;
         }
+        DeclarationParsingResult::Declaration& decl =
+            parsing_result.declarations[0];
         if (parsing_result.first_initializer_loc.IsValid() &&
             (is_strict(language_mode()) || mode == ForEachStatement::ITERATE ||
-             IsLexicalVariableMode(parsing_result.descriptor.mode))) {
+             IsLexicalVariableMode(parsing_result.descriptor.mode) ||
+             !decl.pattern->IsVariableProxy())) {
           if (mode == ForEachStatement::ITERATE) {
             ReportMessageAt(parsing_result.first_initializer_loc,
                             MessageTemplate::kForOfLoopInitializer);
@@ -3693,23 +3686,22 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
           return nullptr;
         }
 
-        DCHECK(parsing_result.declarations.length() == 1);
         Block* init_block = nullptr;
 
         // special case for legacy for (var/const x =.... in)
         if (!IsLexicalVariableMode(parsing_result.descriptor.mode) &&
-            parsing_result.declarations[0].initializer != nullptr) {
+            decl.pattern->IsVariableProxy() && decl.initializer != nullptr) {
+          const AstRawString* name =
+              decl.pattern->AsVariableProxy()->raw_name();
           VariableProxy* single_var = scope_->NewUnresolved(
-              factory(), parsing_result.SingleName(), Variable::NORMAL,
-              each_beg_pos, each_end_pos);
+              factory(), name, Variable::NORMAL, each_beg_pos, each_end_pos);
           init_block = factory()->NewBlock(
               nullptr, 2, true, parsing_result.descriptor.declaration_pos);
           init_block->statements()->Add(
               factory()->NewExpressionStatement(
-                  factory()->NewAssignment(
-                      Token::ASSIGN, single_var,
-                      parsing_result.declarations[0].initializer,
-                      RelocInfo::kNoPosition),
+                  factory()->NewAssignment(Token::ASSIGN, single_var,
+                                           decl.initializer,
+                                           RelocInfo::kNoPosition),
                   RelocInfo::kNoPosition),
               zone());
         }
@@ -3752,9 +3744,6 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         auto each_initialization_block =
             factory()->NewBlock(nullptr, 1, true, RelocInfo::kNoPosition);
         {
-          DCHECK(parsing_result.declarations.length() == 1);
-          DeclarationParsingResult::Declaration decl =
-              parsing_result.declarations[0];
           auto descriptor = parsing_result.descriptor;
           descriptor.declaration_pos = RelocInfo::kNoPosition;
           descriptor.initialization_pos = RelocInfo::kNoPosition;

src/parser.h

@@ -1020,7 +1020,6 @@ class Parser : public ParserBase<ParserTraits> {
 
     Block* BuildInitializationBlock(ZoneList<const AstRawString*>* names,
                                     bool* ok);
-    const AstRawString* SingleName() const;
 
     DeclarationDescriptor descriptor;
     List<Declaration> declarations;

src/preparser.cc

@@ -495,8 +495,8 @@ PreParser::Statement PreParser::ParseVariableStatement(
   // VariableStatement ::
   //   VariableDeclarations ';'
 
-  Statement result = ParseVariableDeclarations(var_context, nullptr, nullptr,
-                                               nullptr, CHECK_OK);
+  Statement result = ParseVariableDeclarations(
+      var_context, nullptr, nullptr, nullptr, nullptr, nullptr, CHECK_OK);
   ExpectSemicolon(CHECK_OK);
   return result;
 }
@@ -508,9 +508,9 @@ PreParser::Statement PreParser::ParseVariableStatement(
 // to initialize it properly. This mechanism is also used for the parsing
 // of 'for-in' loops.
 PreParser::Statement PreParser::ParseVariableDeclarations(
-    VariableDeclarationContext var_context, int* num_decl,
-    Scanner::Location* first_initializer_loc, Scanner::Location* bindings_loc,
-    bool* ok) {
+    VariableDeclarationContext var_context, int* num_decl, bool* is_lexical,
+    bool* is_binding_pattern, Scanner::Location* first_initializer_loc,
+    Scanner::Location* bindings_loc, bool* ok) {
   // VariableDeclarations ::
   //   ('var' | 'const') (Identifier ('=' AssignmentExpression)?)+[',']
   //
@@ -526,6 +526,7 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
   //   BindingPattern '=' AssignmentExpression
   bool require_initializer = false;
   bool lexical = false;
+  bool is_pattern = false;
   if (peek() == Token::VAR) {
     if (is_strong(language_mode())) {
       Scanner::Location location = scanner()->peek_location();
@@ -589,7 +590,7 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
       }
     }
 
-    bool is_pattern = pattern.IsObjectLiteral() || pattern.IsArrayLiteral();
+    is_pattern = pattern.IsObjectLiteral() || pattern.IsArrayLiteral();
 
     bool is_for_iteration_variable =
         var_context == kForStatement &&
@@ -623,7 +624,9 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
         Scanner::Location(bindings_start, scanner()->location().end_pos);
   }
 
-  if (num_decl != NULL) *num_decl = nvars;
+  if (num_decl != nullptr) *num_decl = nvars;
+  if (is_lexical != nullptr) *is_lexical = lexical;
+  if (is_binding_pattern != nullptr) *is_binding_pattern = is_pattern;
   return Statement::Default();
 }
 
@@ -912,11 +915,13 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
     if (peek() == Token::VAR || (peek() == Token::CONST && allow_const()) ||
         (peek() == Token::LET && IsNextLetKeyword())) {
       int decl_count;
+      bool is_lexical;
+      bool is_binding_pattern;
       Scanner::Location first_initializer_loc = Scanner::Location::invalid();
       Scanner::Location bindings_loc = Scanner::Location::invalid();
-      ParseVariableDeclarations(kForStatement, &decl_count,
-                                &first_initializer_loc, &bindings_loc,
-                                CHECK_OK);
+      ParseVariableDeclarations(kForStatement, &decl_count, &is_lexical,
+                                &is_binding_pattern, &first_initializer_loc,
+                                &bindings_loc, CHECK_OK);
       bool accept_IN = decl_count >= 1;
       if (accept_IN && CheckInOrOf(&mode, ok)) {
         if (!*ok) return Statement::Default();
@@ -930,7 +935,8 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
           return Statement::Default();
         }
         if (first_initializer_loc.IsValid() &&
-            (is_strict(language_mode()) || mode == ForEachStatement::ITERATE)) {
+            (is_strict(language_mode()) || mode == ForEachStatement::ITERATE ||
+             is_lexical || is_binding_pattern)) {
           if (mode == ForEachStatement::ITERATE) {
             ReportMessageAt(first_initializer_loc,
                             MessageTemplate::kForOfLoopInitializer);

src/preparser.h

@@ -1845,7 +1845,8 @@ class PreParser : public ParserBase<PreParserTraits> {
   Statement ParseVariableStatement(VariableDeclarationContext var_context,
                                    bool* ok);
   Statement ParseVariableDeclarations(VariableDeclarationContext var_context,
-                                      int* num_decl,
+                                      int* num_decl, bool* is_lexical,
+                                      bool* is_binding_pattern,
                                       Scanner::Location* first_initializer_loc,
                                       Scanner::Location* bindings_loc,
                                       bool* ok);

test/message/for-in-loop-initializers-destructuring.js

+// 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.
+//
+// Flags: --harmony-destructuring-bind
+
+function f() {
+  for (var [x, y] = {} in {});
+}

test/message/for-in-loop-initializers-destructuring.out

+*%(basename)s:8: SyntaxError: for-in loop variable declaration may not have an initializer.
+  for (var [x, y] = {} in {});
+                ^^^^^^
+SyntaxError: for-in loop variable declaration may not have an initializer.