Fix for temporaries in parameter initializers

This patch introduces a mechanism for changing the scope of temporary
variables, which is necessary for rewriting arrow parameter
initializers.

It also fixes a potential bug in AstExpressionVisitor, which did not
visit the automatically generated members of ForEachStatement.

Fixes test/mjsunit/harmony/regress/regress-4658.js

R=rossberg@chromium.org
BUG=v8:4658
LOG=N

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

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

src/ast/ast-expression-visitor.cc

@@ -171,6 +171,11 @@ void AstExpressionVisitor::VisitForInStatement(ForInStatement* stmt) {
 
 void AstExpressionVisitor::VisitForOfStatement(ForOfStatement* stmt) {
   RECURSE(Visit(stmt->iterable()));
+  RECURSE(Visit(stmt->each()));
+  RECURSE(Visit(stmt->assign_iterator()));
+  RECURSE(Visit(stmt->next_result()));
+  RECURSE(Visit(stmt->result_done()));
+  RECURSE(Visit(stmt->assign_each()));
   RECURSE(Visit(stmt->body()));
 }
 

src/ast/scopes.cc

@@ -575,11 +575,24 @@ Variable* Scope::NewTemporary(const AstRawString* name) {
                                        TEMPORARY,
                                        Variable::NORMAL,
                                        kCreatedInitialized);
-  scope->temps_.Add(var, zone());
+  scope->AddTemporary(var);
   return var;
 }
 
 
+bool Scope::RemoveTemporary(Variable* var) {
+  // Most likely (always?) any temporary variable we want to remove
+  // was just added before, so we search backwards.
+  for (int i = temps_.length(); i-- > 0;) {
+    if (temps_[i] == var) {
+      temps_.Remove(i);
+      return true;
+    }
+  }
+  return false;
+}
+
+
 void Scope::AddDeclaration(Declaration* declaration) {
   decls_.Add(declaration, zone());
 }

src/ast/scopes.h

@@ -209,6 +209,15 @@ class Scope: public ZoneObject {
   // names.
   Variable* NewTemporary(const AstRawString* name);
 
+  // Remove a temporary variable. This is for adjusting the scope of
+  // temporaries used when desugaring parameter initializers.
+  bool RemoveTemporary(Variable* var);
+
+  // Adds a temporary variable in this scope's TemporaryScope. This is for
+  // adjusting the scope of temporaries used when desugaring parameter
+  // initializers.
+  void AddTemporary(Variable* var) { temps_.Add(var, zone()); }
+
   // Adds the specific declaration node to the list of declarations in
   // this scope. The declarations are processed as part of entering
   // the scope; see codegen.cc:ProcessDeclarations.

src/ast/variables.h

@@ -37,6 +37,10 @@ class Variable: public ZoneObject {
   // scope is only used to follow the context chain length.
   Scope* scope() const { return scope_; }
 
+  // This is for adjusting the scope of temporaries used when desugaring
+  // parameter initializers.
+  void set_scope(Scope* scope) { scope_ = scope; }
+
   Handle<String> name() const { return name_->string(); }
   const AstRawString* raw_name() const { return name_; }
   VariableMode mode() const { return mode_; }

src/parsing/parameter-initializer-rewriter.cc

@@ -60,8 +60,14 @@ void Rewriter::VisitClassLiteral(ClassLiteral* class_literal) {
 
 
 void Rewriter::VisitVariableProxy(VariableProxy* proxy) {
-  DCHECK(!proxy->is_resolved());
-  if (old_scope_->RemoveUnresolved(proxy)) {
+  if (proxy->is_resolved()) {
+    Variable* var = proxy->var();
+    DCHECK_EQ(var->mode(), TEMPORARY);
+    if (old_scope_->RemoveTemporary(var)) {
+      var->set_scope(new_scope_);
+      new_scope_->AddTemporary(var);
+    }
+  } else if (old_scope_->RemoveUnresolved(proxy)) {
     new_scope_->AddUnresolved(proxy);
   }
 }

test/mjsunit/harmony/regress/regress-4658.js

@@ -0,0 +1,21 @@
+// 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-do-expressions
+
+(function testWithSimpleLoopVariable() {
+  var f = (x, y = (do { var s=0; for (var e of x) s += e; s; })) => y*(y+1);
+  var result = f([1,2,3]);  // core dump here, if not fixed.
+  assertEquals(result, 42);
+})();
+
+(function testWithComplexLoopVariable() {
+  var f = (x, i=x[0]-1, a=[],
+           y = (do { var s=0;
+                     for (a[i] of x) s += a[i++];
+                     s;
+                   })) => y*(a[0]+a[1]*a[2]);
+  var result = f([1,2,3]);  // core dump here, if not fixed.
+  assertEquals(result, 42);
+})();