Revert of [parser] Fix scopes in rewriting of for-of and destructuring...

Revert of [parser] Fix scopes in rewriting of for-of and destructuring assignments. (patchset #6 id:100001 of https://codereview.chromium.org/2520883002/ )

Reason for revert:
Speculative revert: Seems to break jsfunfuzz:
https://build.chromium.org/p/client.v8/builders/V8%20Fuzzer/builds/14385

Original issue's description:
> [parser] Fix scopes in rewriting of for-of and destructuring assignments.
>
> The catch scopes were created with the wrong parent scope.
>
> R=littledan@chromium.org
> BUG=v8:5648
>
> Committed: https://crrev.com/f385268d11d6da9508e481202b39f75f4b56afdd
> Cr-Commit-Position: refs/heads/master@{#41222}

TBR=littledan@chromium.org,verwaest@chromium.org,neis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:5648

Review-Url: https://codereview.chromium.org/2519333005
Cr-Commit-Position: refs/heads/master@{#41228}

src/ast/scopes.cc

@@ -635,36 +635,6 @@ Variable* DeclarationScope::DeclareFunctionVar(const AstRawString* name) {
   return function_;
 }
 
-bool Scope::HasBeenRemoved() const {
-  // TODO(neis): Store this information somewhere instead of calculating it.
-
-  if (is_declaration_scope()) return false;
-  DCHECK(is_block_scope());
-
-  Scope* parent = outer_scope();
-  if (parent == nullptr) {
-    DCHECK(is_script_scope());
-    return false;
-  }
-
-  Scope* sibling = parent->inner_scope();
-  for (; sibling != nullptr; sibling = sibling->sibling()) {
-    if (sibling == this) return false;
-  }
-
-  DCHECK_NULL(inner_scope_);
-  return true;
-}
-
-Scope* Scope::GetUnremovedScope() {
-  Scope* scope = this;
-  while (scope != nullptr && scope->HasBeenRemoved()) {
-    scope = scope->outer_scope();
-  }
-  DCHECK_NOT_NULL(scope);
-  return scope;
-}
-
 Scope* Scope::FinalizeBlockScope() {
   DCHECK(is_block_scope());
 

src/ast/scopes.h

@@ -113,11 +113,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // tree and its children are reparented.
   Scope* FinalizeBlockScope();
 
-  bool HasBeenRemoved() const;
-
-  // Find the first scope that hasn't been removed.
-  Scope* GetUnremovedScope();
-
   // Inserts outer_scope into this scope's scope chain (and removes this
   // from the current outer_scope_'s inner scope list).
   // Assumes outer_scope_ is non-null.

src/parsing/parser.cc

@@ -4332,11 +4332,8 @@ void Parser::RewriteDestructuringAssignments() {
         pair.assignment->AsRewritableExpression();
     DCHECK_NOT_NULL(to_rewrite);
     if (!to_rewrite->is_rewritten()) {
-      // Since this function is called at the end of parsing the program,
-      // pair.scope may already have been removed by FinalizeBlockScope in the
-      // meantime.
-      Scope* scope = pair.scope->GetUnremovedScope();
-      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope);
+      PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite,
+                                                      pair.scope);
     }
   }
 }
@@ -4771,7 +4768,7 @@ Expression* Parser::RewriteYieldStar(Expression* generator,
 
     Block* then = factory()->NewBlock(nullptr, 4 + 1, false, nopos);
     BuildIteratorCloseForCompletion(
-        scope(), then->statements(), var_iterator,
+        then->statements(), var_iterator,
         factory()->NewSmiLiteral(Parser::kNormalCompletion, nopos));
     then->statements()->Add(throw_call, zone());
     check_throw = factory()->NewIfStatement(
@@ -5139,9 +5136,9 @@ void Parser::BuildIteratorClose(ZoneList<Statement*>* statements,
   statements->Add(validate_output, zone());
 }
 
-void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
-                                 Expression* condition, Variable* iter,
-                                 Block* iterator_use, Block* target) {
+void Parser::FinalizeIteratorUse(Variable* completion, Expression* condition,
+                                 Variable* iter, Block* iterator_use,
+                                 Block* target) {
   //
   // This function adds two statements to [target], corresponding to the
   // following code:
@@ -5197,8 +5194,7 @@ void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
   {
     Block* block = factory()->NewBlock(nullptr, 2, true, nopos);
     Expression* proxy = factory()->NewVariableProxy(completion);
-    BuildIteratorCloseForCompletion(use_scope, block->statements(), iter,
-                                    proxy);
+    BuildIteratorCloseForCompletion(block->statements(), iter, proxy);
     DCHECK(block->statements()->length() == 2);
 
     maybe_close = factory()->NewBlock(nullptr, 1, true, nopos);
@@ -5215,7 +5211,7 @@ void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
   // }
   Statement* try_catch;
   {
-    Scope* catch_scope = NewScopeWithParent(use_scope, CATCH_SCOPE);
+    Scope* catch_scope = NewScopeWithParent(scope(), CATCH_SCOPE);
     Variable* catch_variable =
         catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR,
                                   kCreatedInitialized, NORMAL_VARIABLE);
@@ -5255,8 +5251,7 @@ void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
   target->statements()->Add(try_finally, zone());
 }
 
-void Parser::BuildIteratorCloseForCompletion(Scope* scope,
-                                             ZoneList<Statement*>* statements,
+void Parser::BuildIteratorCloseForCompletion(ZoneList<Statement*>* statements,
                                              Variable* iterator,
                                              Expression* completion) {
   //
@@ -5322,7 +5317,7 @@ void Parser::BuildIteratorCloseForCompletion(Scope* scope,
 
     Block* catch_block = factory()->NewBlock(nullptr, 0, false, nopos);
 
-    Scope* catch_scope = NewScopeWithParent(scope, CATCH_SCOPE);
+    Scope* catch_scope = NewScope(CATCH_SCOPE);
     Variable* catch_variable =
         catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR,
                                   kCreatedInitialized, NORMAL_VARIABLE);
@@ -5459,13 +5454,8 @@ Statement* Parser::FinalizeForOfStatement(ForOfStatement* loop,
     Block* try_block = factory()->NewBlock(nullptr, 1, false, nopos);
     try_block->statements()->Add(loop, zone());
 
-    // The scope in which the parser creates this loop.
-    Scope* loop_scope = scope()->outer_scope();
-    DCHECK_EQ(loop_scope->scope_type(), BLOCK_SCOPE);
-    DCHECK_EQ(scope()->scope_type(), BLOCK_SCOPE);
-
-    FinalizeIteratorUse(loop_scope, var_completion, closing_condition,
-                        loop->iterator(), try_block, final_loop);
+    FinalizeIteratorUse(var_completion, closing_condition, loop->iterator(),
+                        try_block, final_loop);
   }
 
   return final_loop;

src/parsing/parser.h

@@ -638,16 +638,14 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                             MessageTemplate::Template message,
                             const AstRawString* arg, int pos);
 
-  void FinalizeIteratorUse(Scope* use_scope, Variable* completion,
-                           Expression* condition, Variable* iter,
-                           Block* iterator_use, Block* result);
+  void FinalizeIteratorUse(Variable* completion, Expression* condition,
+                           Variable* iter, Block* iterator_use, Block* result);
 
   Statement* FinalizeForOfStatement(ForOfStatement* loop, Variable* completion,
                                     int pos);
   void BuildIteratorClose(ZoneList<Statement*>* statements, Variable* iterator,
                           Variable* input, Variable* output);
-  void BuildIteratorCloseForCompletion(Scope* scope,
-                                       ZoneList<Statement*>* statements,
+  void BuildIteratorCloseForCompletion(ZoneList<Statement*>* statements,
                                        Variable* iterator,
                                        Expression* completion);
   Statement* CheckCallable(Variable* var, Expression* error, int pos);

src/parsing/pattern-rewriter.cc

@@ -37,12 +37,11 @@ void Parser::PatternRewriter::DeclareAndInitializeVariables(
 
 void Parser::PatternRewriter::RewriteDestructuringAssignment(
     Parser* parser, RewritableExpression* to_rewrite, Scope* scope) {
-  DCHECK(!scope->HasBeenRemoved());
+  PatternRewriter rewriter;
+
   DCHECK(!to_rewrite->is_rewritten());
 
   bool ok = true;
-
-  PatternRewriter rewriter;
   rewriter.scope_ = scope;
   rewriter.parser_ = parser;
   rewriter.context_ = ASSIGNMENT;
@@ -587,9 +586,8 @@ void Parser::PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
 
   Expression* closing_condition = factory()->NewUnaryOperation(
       Token::NOT, factory()->NewVariableProxy(done), nopos);
-
-  parser_->FinalizeIteratorUse(scope(), completion, closing_condition, iterator,
-                               block_, target);
+  parser_->FinalizeIteratorUse(completion, closing_condition, iterator, block_,
+                               target);
   block_ = target;
 }
 

test/mjsunit/regress/regress-5648.js

-// Copyright 2016 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.
-
-
-var iter = {}
-iter[Symbol.iterator] = () => ({
-  next: () => ({}),
-  return: () => {throw 666}
-});
-
-
-function* foo() {
-  for (let x of iter) {throw 42}
-}
-assertThrowsEquals(() => foo().next(), 42);
-
-
-function* bar() {
-  let x;
-  { let gaga = () => {x};
-    [[x]] = iter;
-  }
-}
-assertThrows(() => bar().next(), TypeError);
-
-
-function baz() {
-  let x;
-  { let gaga = () => {x};
-    let gugu = () => {gaga};
-    [[x]] = iter;
-  }
-}
-assertThrows(baz, TypeError);