Revert of [es6] support AssignmentPattern as LHS in for-in/of loops (patchset #9 id:280001 of https://codereview.chromium.org/1508933004/ )

Reason for revert: Hits unreachable code (found by fuzzer). Example crasher: "for(();;);" Original issue's description: > [es6] support AssignmentPattern as LHS in for-in/of loops > > BUG=v8:811, v8:4599 > LOG=N > R=adamk@chromium.org, rossberg@chromium.org > > Committed: https://crrev.com/e47bdb775564b2cd8365047425898ab4274190a6 > Cr-Commit-Position: refs/heads/master@{#32773} TBR=rossberg@chromium.org,caitpotter88@gmail.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:811, v8:4599 Review URL: https://codereview.chromium.org/1511773009 Cr-Commit-Position: refs/heads/master@{#32774}
2015-12-10 17:59:28 -08:00 · 2015-12-10 17:59:28 -08:00 · 8b968b70e9
commit 8b968b70e9
parent e47bdb7755
7 changed files with 37 additions and 160 deletions
--- a/src/ast/ast.cc
+++ b/src/ast/ast.cc
@ -137,10 +137,6 @@ static void AssignVectorSlots(Expression* expr, FeedbackVectorSpec* spec,
 void ForEachStatement::AssignFeedbackVectorSlots(
    Isolate* isolate, FeedbackVectorSpec* spec,
    FeedbackVectorSlotCache* cache) {
-  // TODO(caitp): for-of statements do not make use of this feedback slot.
-  // The each_slot_ should be specific to ForInStatement, and this work moved
-  // there.
-  if (IsForOfStatement()) return;
  AssignVectorSlots(each(), spec, &each_slot_);
 }

--- a/src/parsing/parser.cc
+++ b/src/parsing/parser.cc
@ -3322,10 +3322,9 @@ Expression* Parser::BuildIteratorNextResult(Expression* iterator,


 void Parser::InitializeForEachStatement(ForEachStatement* stmt,
-                                        Expression* each, Expression* subject,
-                                        Statement* body,
-                                        bool is_destructuring) {
-  DCHECK(!is_destructuring || allow_harmony_destructuring_assignment());
+                                        Expression* each,
+                                        Expression* subject,
+                                        Statement* body) {
  ForOfStatement* for_of = stmt->AsForOfStatement();

  if (for_of != NULL) {
@ -3376,10 +3375,6 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
          result_proxy, value_literal, RelocInfo::kNoPosition);
      assign_each = factory()->NewAssignment(Token::ASSIGN, each, result_value,
                                             RelocInfo::kNoPosition);
-      if (is_destructuring) {
-        assign_each = PatternRewriter::RewriteDestructuringAssignment(
-            this, assign_each->AsAssignment(), scope_);
-      }
    }

    for_of->Initialize(each, subject, body,
@ -3388,23 +3383,6 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
                       result_done,
                       assign_each);
  } else {
-    if (is_destructuring) {
-      Variable* temp =
-          scope_->NewTemporary(ast_value_factory()->empty_string());
-      VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
-      Expression* assign_each = PatternRewriter::RewriteDestructuringAssignment(
-          this, factory()->NewAssignment(Token::ASSIGN, each, temp_proxy,
-                                         RelocInfo::kNoPosition),
-          scope_);
-      auto block =
-          factory()->NewBlock(nullptr, 2, false, RelocInfo::kNoPosition);
-      block->statements()->Add(factory()->NewExpressionStatement(
-                                   assign_each, RelocInfo::kNoPosition),
-                               zone());
-      block->statements()->Add(body, zone());
-      body = block;
-      each = factory()->NewVariableProxy(temp);
-    }
    stmt->Initialize(each, subject, body);
  }
 }
@ -3789,8 +3767,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
        body_block->statements()->Add(body, zone());
        VariableProxy* temp_proxy =
            factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
-        InitializeForEachStatement(loop, temp_proxy, enumerable, body_block,
-                                   false);
+        InitializeForEachStatement(loop, temp_proxy, enumerable, body_block);
        scope_ = for_scope;
        body_scope->set_end_position(scanner()->location().end_pos);
        body_scope = body_scope->FinalizeBlockScope();
@ -3841,8 +3818,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
      }
    } else {
      int lhs_beg_pos = peek_position();
-      ExpressionClassifier classifier;
-      Expression* expression = ParseExpression(false, &classifier, CHECK_OK);
+      Expression* expression = ParseExpression(false, CHECK_OK);
      int lhs_end_pos = scanner()->location().end_pos;
      ForEachStatement::VisitMode mode;
      is_let_identifier_expression =
@ -3852,17 +3828,9 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,

      if (CheckInOrOf(&mode, ok)) {
        if (!*ok) return nullptr;
-        bool is_destructuring =
-            allow_harmony_destructuring_assignment() &&
-            (expression->IsArrayLiteral() || expression->IsObjectLiteral());
-        if (is_destructuring) {
-          ValidateAssignmentPattern(&classifier, CHECK_OK);
-        } else {
-          ValidateExpression(&classifier, CHECK_OK);
-          expression = this->CheckAndRewriteReferenceExpression(
-              expression, lhs_beg_pos, lhs_end_pos,
-              MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);
-        }
+        expression = this->CheckAndRewriteReferenceExpression(
+            expression, lhs_beg_pos, lhs_end_pos,
+            MessageTemplate::kInvalidLhsInFor, kSyntaxError, CHECK_OK);

        ForEachStatement* loop =
            factory()->NewForEachStatement(mode, labels, stmt_pos);
@ -3883,8 +3851,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
            factory()->NewBlock(NULL, 1, false, RelocInfo::kNoPosition);
        Statement* body = ParseSubStatement(NULL, CHECK_OK);
        block->statements()->Add(body, zone());
-        InitializeForEachStatement(loop, expression, enumerable, block,
-                                   is_destructuring);
+        InitializeForEachStatement(loop, expression, enumerable, block);
        scope_ = saved_scope;
        body_scope->set_end_position(scanner()->location().end_pos);
        body_scope = body_scope->FinalizeBlockScope();
@ -4584,8 +4551,10 @@ class InitializerRewriter : public AstExpressionVisitor {
        expr->AsRewritableAssignmentExpression();
    if (to_rewrite == nullptr || to_rewrite->is_rewritten()) return;

+    bool ok = true;
    Parser::PatternRewriter::RewriteDestructuringAssignment(parser_, to_rewrite,
-                                                            scope_);
+                                                            scope_, &ok);
+    DCHECK(ok);
  }

 private:
@ -6554,7 +6523,10 @@ void Parser::RewriteDestructuringAssignments() {
    Scope* scope = pair.scope;
    DCHECK_NOT_NULL(to_rewrite);
    if (!to_rewrite->is_rewritten()) {
-      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope);
+      bool ok = true;
+      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope,
+                                                      &ok);
+      DCHECK(ok);
    }
  }
 }
--- a/src/parsing/parser.h
+++ b/src/parsing/parser.h
@ -1047,11 +1047,8 @@ class Parser : public ParserBase<ParserTraits> {
        ZoneList<const AstRawString*>* names, bool* ok);

    static void RewriteDestructuringAssignment(
-        Parser* parser, RewritableAssignmentExpression* expr, Scope* Scope);
-
-    static Expression* RewriteDestructuringAssignment(Parser* parser,
-                                                      Assignment* assignment,
-                                                      Scope* scope);
+        Parser* parser, RewritableAssignmentExpression* expr, Scope* Scope,
+        bool* ok);

    void set_initializer_position(int pos) { initializer_position_ = pos; }

@ -1147,9 +1144,10 @@ class Parser : public ParserBase<ParserTraits> {


  // Initialize the components of a for-in / for-of statement.
-  void InitializeForEachStatement(ForEachStatement* stmt, Expression* each,
-                                  Expression* subject, Statement* body,
-                                  bool is_destructuring);
+  void InitializeForEachStatement(ForEachStatement* stmt,
+                                  Expression* each,
+                                  Expression* subject,
+                                  Statement* body);
  Statement* DesugarLexicalBindingsInForStatement(
      Scope* inner_scope, bool is_const, ZoneList<const AstRawString*>* names,
      ForStatement* loop, Statement* init, Expression* cond, Statement* next,
--- a/src/parsing/pattern-rewriter.cc
+++ b/src/parsing/pattern-rewriter.cc
@ -32,12 +32,12 @@ void Parser::PatternRewriter::DeclareAndInitializeVariables(


 void Parser::PatternRewriter::RewriteDestructuringAssignment(
-    Parser* parser, RewritableAssignmentExpression* to_rewrite, Scope* scope) {
+    Parser* parser, RewritableAssignmentExpression* to_rewrite, Scope* scope,
+    bool* ok) {
  PatternRewriter rewriter;

  DCHECK(!to_rewrite->is_rewritten());

-  bool ok = true;
  rewriter.scope_ = scope;
  rewriter.parser_ = parser;
  rewriter.context_ = ASSIGNMENT;
@ -45,21 +45,9 @@ void Parser::PatternRewriter::RewriteDestructuringAssignment(
  rewriter.block_ = nullptr;
  rewriter.descriptor_ = nullptr;
  rewriter.names_ = nullptr;
-  rewriter.ok_ = &ok;
+  rewriter.ok_ = ok;

  rewriter.RecurseIntoSubpattern(rewriter.pattern_, nullptr);
-  DCHECK(ok);
-}
-
-
-Expression* Parser::PatternRewriter::RewriteDestructuringAssignment(
-    Parser* parser, Assignment* assignment, Scope* scope) {
-  DCHECK_NOT_NULL(assignment);
-  DCHECK_EQ(Token::ASSIGN, assignment->op());
-  auto to_rewrite =
-      parser->factory()->NewRewritableAssignmentExpression(assignment);
-  RewriteDestructuringAssignment(parser, to_rewrite, scope);
-  return to_rewrite->expression();
 }


--- a/src/parsing/preparser.cc
+++ b/src/parsing/preparser.cc
@ -956,24 +956,15 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
      }
    } else {
      int lhs_beg_pos = peek_position();
-      ExpressionClassifier classifier;
-      Expression lhs = ParseExpression(false, &classifier, CHECK_OK);
+      Expression lhs = ParseExpression(false, CHECK_OK);
      int lhs_end_pos = scanner()->location().end_pos;
      is_let_identifier_expression =
          lhs.IsIdentifier() && lhs.AsIdentifier().IsLet();
      if (CheckInOrOf(&mode, ok)) {
        if (!*ok) return Statement::Default();
-        bool is_destructuring =
-            allow_harmony_destructuring_assignment() &&
-            (lhs->IsArrayLiteral() || lhs->IsObjectLiteral());
-        if (is_destructuring) {
-          ValidateAssignmentPattern(&classifier, CHECK_OK);
-        } else {
-          ValidateExpression(&classifier, CHECK_OK);
-          lhs = CheckAndRewriteReferenceExpression(
-              lhs, lhs_beg_pos, lhs_end_pos, MessageTemplate::kInvalidLhsInFor,
-              kSyntaxError, CHECK_OK);
-        }
+        lhs = CheckAndRewriteReferenceExpression(
+            lhs, lhs_beg_pos, lhs_end_pos, MessageTemplate::kInvalidLhsInFor,
+            kSyntaxError, CHECK_OK);
        ParseExpression(true, CHECK_OK);
        Expect(Token::RPAREN, CHECK_OK);
        ParseSubStatement(CHECK_OK);
--- a/test/cctest/test-parsing.cc
+++ b/test/cctest/test-parsing.cc
@ -6828,25 +6828,6 @@ TEST(DestructuringAssignmentPositiveTests) {
      {"'use strict'; let x, y, z; for (x of ", " = {});"},
      {"var x, y, z; for (x in ", " = {});"},
      {"var x, y, z; for (x of ", " = {});"},
-      {"var x, y, z; for (", " in {});"},
-      {"var x, y, z; for (", " of {});"},
-      {"'use strict'; var x, y, z; for (", " in {});"},
-      {"'use strict'; var x, y, z; for (", " of {});"},
-      {NULL, NULL}};
-
-  const char* mixed_assignments_context_data[][2] = {
-      {"'use strict'; let x, y, z; (", " = z = {});"},
-      {"var x, y, z; (", " = z = {});"},
-      {"'use strict'; let x, y, z; (x = ", " = z = {});"},
-      {"var x, y, z; (x = ", " = z = {});"},
-      {"'use strict'; let x, y, z; for (x in ", " = z = {});"},
-      {"'use strict'; let x, y, z; for (x in x = ", " = z = {});"},
-      {"'use strict'; let x, y, z; for (x of ", " = z = {});"},
-      {"'use strict'; let x, y, z; for (x of x = ", " = z = {});"},
-      {"var x, y, z; for (x in ", " = z = {});"},
-      {"var x, y, z; for (x in x = ", " = z = {});"},
-      {"var x, y, z; for (x of ", " = z = {});"},
-      {"var x, y, z; for (x of x = ", " = z = {});"},
      {NULL, NULL}};

  // clang-format off
@ -6920,6 +6901,8 @@ TEST(DestructuringAssignmentPositiveTests) {
    "[ [ foo()[x] = 10 ] = {} ]",
    "[ [ x.y = 10 ] = {} ]",
    "[ [ x[y] = 10 ] = {} ]",
+
+    "{ x : y }",
    "{ x : y = 1 }",
    "{ x }",
    "{ x, y, z }",
@ -6962,8 +6945,12 @@ TEST(DestructuringAssignmentPositiveTests) {
    "[...x]",
    "[x,y,...z]",
    "[x,,...z]",
-    "{ x: y }",
-    "[x, y]",
+    "{ x: y } = z",
+    "[x, y] = z",
+    "{ x: y } = { z }",
+    "[x, y] = { z }",
+    "{ x: y } = [ z ]",
+    "[x, y] = [ z ]",
    "[((x, y) => z).x]",
    "{x: ((y, z) => z).x}",
    "[((x, y) => z)['x']]",
@ -6979,9 +6966,6 @@ TEST(DestructuringAssignmentPositiveTests) {
  RunParserSyncTest(context_data, data, kSuccess, NULL, 0, always_flags,
                    arraysize(always_flags));

-  RunParserSyncTest(mixed_assignments_context_data, data, kSuccess, NULL, 0,
-                    always_flags, arraysize(always_flags));
-
  const char* empty_context_data[][2] = {
      {"'use strict';", ""}, {"", ""}, {NULL, NULL}};

--- a/test/mjsunit/harmony/destructuring-assignment.js
+++ b/test/mjsunit/harmony/destructuring-assignment.js
@ -428,55 +428,3 @@ assertEquals(oz, [1, 2, 3, 4, 5]);
  assertThrows(() => { ({ a: [ c ] } = { a: [ "nope!" ] }); }, TypeError);
  assertEquals("untouchable", c);
 })();
-
-(function testForIn() {
-  var log = [];
-  var x = {};
-  var object = {
-    "Apenguin": 1,
-    "\u{1F382}cake": 2,
-    "Bpuppy": 3,
-    "Cspork": 4
-  };
-  for ([x.firstLetter, ...x.rest] in object) {
-    if (x.firstLetter === "A") {
-      assertEquals(["p", "e", "n", "g", "u", "i", "n"], x.rest);
-      continue;
-    }
-    if (x.firstLetter === "C") {
-      assertEquals(["s", "p", "o", "r", "k"], x.rest);
-      break;
-    }
-    log.push({ firstLetter: x.firstLetter, rest: x.rest });
-  }
-  assertEquals([
-    { firstLetter: "\u{1F382}", rest: ["c", "a", "k", "e"] },
-    { firstLetter: "B", rest: ["p", "u", "p", "p", "y"] },
-  ], log);
-})();
-
-(function testForOf() {
-  var log = [];
-  var x = {};
-  var names = [
-    "Apenguin",
-    "\u{1F382}cake",
-    "Bpuppy",
-    "Cspork"
-  ];
-  for ([x.firstLetter, ...x.rest] of names) {
-    if (x.firstLetter === "A") {
-      assertEquals(["p", "e", "n", "g", "u", "i", "n"], x.rest);
-      continue;
-    }
-    if (x.firstLetter === "C") {
-      assertEquals(["s", "p", "o", "r", "k"], x.rest);
-      break;
-    }
-    log.push({ firstLetter: x.firstLetter, rest: x.rest });
-  }
-  assertEquals([
-    { firstLetter: "\u{1F382}", rest: ["c", "a", "k", "e"] },
-    { firstLetter: "B", rest: ["p", "u", "p", "p", "y"] },
-  ], log);
-})();