[parser] Fix arrow funcs w/ destructuring params again. [Alternative fix]

What happened:
- When rewriting in DoParseFunction, the relevant function scope is no longer in
the scope stack.
- The correct scope is given to the PatternRewriter.
- PatternRewriter called to Parser::BuildIteratorCloseForCompletion.
- BuildIteratorCloseForCompletion would just call NewTemporary (which creates
a new temporary in Parser's current scope) instead of using the scope passed to
it and calling NewTemporary on it.
- Normally this went unnoticed, since it doesn't matter that much where the
temporary is.
- But in the lazy arrow func case, the Parser's scope at that point was the
already-resolved outer scope, and a DCHECK detected this problem.

Kudos & thanks to verwaest@ for a debugging session :)

BUG=chromium:761831

Change-Id: I1e8474ce927be0330f4ba4efc0fc08fdcc328809
Reviewed-on: https://chromium-review.googlesource.com/650297
Commit-Queue: Marja Hölttä <marja@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47877}

src/parsing/parser.cc

@@ -1759,7 +1759,7 @@ void Parser::ParseAndRewriteAsyncGeneratorFunctionBody(
   try_block->statements()->Add(final_return, zone());
 
   // For AsyncGenerators, a top-level catch block will reject the Promise.
-  Scope* catch_scope = NewHiddenCatchScopeWithParent(scope());
+  Scope* catch_scope = NewHiddenCatchScope();
 
   ZoneList<Expression*>* reject_args =
       new (zone()) ZoneList<Expression*>(2, zone());
@@ -2864,7 +2864,7 @@ class InitializerRewriter final
   // Just rewrite destructuring assignments wrapped in RewritableExpressions.
   void VisitRewritableExpression(RewritableExpression* to_rewrite) {
     if (to_rewrite->is_rewritten()) return;
-    parser_->RewriteDestructuringAssignment(to_rewrite, parser_->scope());
+    parser_->RewriteDestructuringAssignment(to_rewrite);
     AstTraversalVisitor::VisitRewritableExpression(to_rewrite);
   }
 
@@ -2952,8 +2952,8 @@ Block* Parser::BuildParameterInitializationBlock(
   return init_block;
 }
 
-Scope* Parser::NewHiddenCatchScopeWithParent(Scope* parent) {
-  Scope* catch_scope = NewScopeWithParent(parent, CATCH_SCOPE);
+Scope* Parser::NewHiddenCatchScope() {
+  Scope* catch_scope = NewScopeWithParent(scope(), CATCH_SCOPE);
   catch_scope->DeclareLocal(ast_value_factory()->dot_catch_string(), VAR);
   catch_scope->set_is_hidden();
   return catch_scope;
@@ -2986,7 +2986,7 @@ Block* Parser::BuildRejectPromiseOnException(Block* inner_block) {
   result->statements()->Add(set_promise, zone());
 
   // catch (.catch) { return %RejectPromise(.promise, .catch), .promise }
-  Scope* catch_scope = NewHiddenCatchScopeWithParent(scope());
+  Scope* catch_scope = NewHiddenCatchScope();
 
   Expression* promise_reject = BuildRejectPromise(
       factory()->NewVariableProxy(catch_scope->catch_variable()),
@@ -3781,7 +3781,8 @@ void Parser::RewriteDestructuringAssignments() {
       // pair.scope may already have been removed by FinalizeBlockScope in the
       // meantime.
       Scope* scope = pair.scope->GetUnremovedScope();
-      RewriteDestructuringAssignment(to_rewrite, scope);
+      BlockState block_state(&scope_, scope);
+      RewriteDestructuringAssignment(to_rewrite);
     }
   }
 }
@@ -4124,10 +4125,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,
-                                 IteratorType type) {
+void Parser::FinalizeIteratorUse(Variable* completion, Expression* condition,
+                                 Variable* iter, Block* iterator_use,
+                                 Block* target, IteratorType type) {
   //
   // This function adds two statements to [target], corresponding to the
   // following code:
@@ -4183,8 +4183,7 @@ void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
   {
     Block* block = factory()->NewBlock(2, true);
     Expression* proxy = factory()->NewVariableProxy(completion);
-    BuildIteratorCloseForCompletion(use_scope, block->statements(), iter, proxy,
-                                    type);
+    BuildIteratorCloseForCompletion(block->statements(), iter, proxy, type);
     DCHECK(block->statements()->length() == 2);
 
     maybe_close = IgnoreCompletion(factory()->NewIfStatement(
@@ -4198,7 +4197,7 @@ void Parser::FinalizeIteratorUse(Scope* use_scope, Variable* completion,
   // }
   Statement* try_catch;
   {
-    Scope* catch_scope = NewHiddenCatchScopeWithParent(use_scope);
+    Scope* catch_scope = NewHiddenCatchScope();
 
     Statement* rethrow;
     // We use %ReThrow rather than the ordinary throw because we want to
@@ -4235,8 +4234,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,
                                              IteratorType type) {
@@ -4314,7 +4312,7 @@ void Parser::BuildIteratorCloseForCompletion(Scope* scope,
                                  zone());
 
     Block* catch_block = factory()->NewBlock(0, false);
-    Scope* catch_scope = NewHiddenCatchScopeWithParent(scope);
+    Scope* catch_scope = NewHiddenCatchScope();
     try_call_return = factory()->NewTryCatchStatement(try_block, catch_scope,
                                                       catch_block, nopos);
   }
@@ -4445,8 +4443,8 @@ Statement* Parser::FinalizeForOfStatement(ForOfStatement* loop,
     Block* try_block = factory()->NewBlock(1, false);
     try_block->statements()->Add(loop, zone());
 
-    FinalizeIteratorUse(scope(), var_completion, closing_condition,
-                        loop->iterator(), try_block, final_loop, type);
+    FinalizeIteratorUse(var_completion, closing_condition, loop->iterator(),
+                        try_block, final_loop, type);
   }
 
   return final_loop;

src/parsing/parser.h

@@ -377,7 +377,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   V8_INLINE Block* IgnoreCompletion(Statement* statement);
 
-  V8_INLINE Scope* NewHiddenCatchScopeWithParent(Scope* parent);
+  V8_INLINE Scope* NewHiddenCatchScope();
 
   // PatternRewriter and associated methods defined in pattern-rewriter.cc.
   friend class PatternRewriter;
@@ -385,7 +385,7 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
       Block* block, const DeclarationDescriptor* declaration_descriptor,
       const DeclarationParsingResult::Declaration* declaration,
       ZoneList<const AstRawString*>* names, bool* ok);
-  void RewriteDestructuringAssignment(RewritableExpression* expr, Scope* Scope);
+  void RewriteDestructuringAssignment(RewritableExpression* expr);
   Expression* RewriteDestructuringAssignment(Assignment* assignment);
 
   // [if (IteratorType == kAsync)]
@@ -583,17 +583,15 @@ 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,
                            IteratorType type);
 
   Statement* FinalizeForOfStatement(ForOfStatement* loop, Variable* completion,
                                     IteratorType type, int pos);
   void BuildIteratorClose(ZoneList<Statement*>* statements, Variable* iterator,
                           Variable* input, Variable* output, IteratorType type);
-  void BuildIteratorCloseForCompletion(Scope* scope,
-                                       ZoneList<Statement*>* statements,
+  void BuildIteratorCloseForCompletion(ZoneList<Statement*>* statements,
                                        Variable* iterator,
                                        Expression* completion,
                                        IteratorType type);

src/parsing/pattern-rewriter.cc

@@ -124,16 +124,15 @@ void Parser::DeclareAndInitializeVariables(
       this, block, declaration_descriptor, declaration, names, ok);
 }
 
-void Parser::RewriteDestructuringAssignment(RewritableExpression* to_rewrite,
-                                            Scope* scope) {
-  PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope);
+void Parser::RewriteDestructuringAssignment(RewritableExpression* to_rewrite) {
+  PatternRewriter::RewriteDestructuringAssignment(this, to_rewrite, scope());
 }
 
 Expression* Parser::RewriteDestructuringAssignment(Assignment* assignment) {
   DCHECK_NOT_NULL(assignment);
   DCHECK_EQ(Token::ASSIGN, assignment->op());
   auto to_rewrite = factory()->NewRewritableExpression(assignment);
-  RewriteDestructuringAssignment(to_rewrite, scope());
+  RewriteDestructuringAssignment(to_rewrite);
   return to_rewrite->expression();
 }
 
@@ -667,8 +666,8 @@ void PatternRewriter::VisitArrayLiteral(ArrayLiteral* node,
   Expression* closing_condition = factory()->NewUnaryOperation(
       Token::NOT, factory()->NewVariableProxy(done), nopos);
 
-  parser_->FinalizeIteratorUse(scope(), completion, closing_condition, iterator,
-                               block_, target, IteratorType::kNormal);
+  parser_->FinalizeIteratorUse(completion, closing_condition, iterator, block_,
+                               target, IteratorType::kNormal);
   block_ = target;
 }
 

test/mjsunit/regress/regress-761831.js

+// Copyright 2017 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.
+
+function OrigReproCase() {
+  assertThrows('var f = ([x=[a=undefined]=[]]) => {}; f();', TypeError);
+}
+OrigReproCase();
+
+function SimpleReproCase() {
+  assertThrows('var f = ([x=[]=[]]) => {}; f()', TypeError);
+}
+SimpleReproCase();