[es6] Make sure temporaries are not allocated in block scope

While at it, remove the notion of INTERNAL variables.

@caitp: Took some parts from your CL, since I was blocked on the temp scope bug.

R=mstarzinger@chromium.org
BUG=512574
LOG=N

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

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

src/contexts.cc

@@ -140,7 +140,6 @@ static void GetAttributesAndBindingFlags(VariableMode mode,
                                          PropertyAttributes* attributes,
                                          BindingFlags* binding_flags) {
   switch (mode) {
-    case INTERNAL:  // Fall through.
     case VAR:
       *attributes = NONE;
       *binding_flags = MUTABLE_IS_INITIALIZED;

src/globals.h

@@ -810,9 +810,6 @@ enum VariableMode {
   IMPORT,          // declared via 'import' declarations (last lexical)
 
   // Variables introduced by the compiler:
-  INTERNAL,        // like VAR, but not user-visible (may or may not
-                   // be in a context)
-
   TEMPORARY,       // temporary variables (not user-visible), stack-allocated
                    // unless the scope as a whole has forced context allocation
 

src/parser.cc

@@ -2827,7 +2827,7 @@ Statement* Parser::ParseReturnStatement(bool* ok) {
       //
       //   return (temp = expr) === undefined ? this :
       //       %_IsSpecObject(temp) ? temp : throw new TypeError(...);
-      Variable* temp = scope_->DeclarationScope()->NewTemporary(
+      Variable* temp = scope_->NewTemporary(
           ast_value_factory()->empty_string());
       Assignment* assign = factory()->NewAssignment(
           Token::ASSIGN, factory()->NewVariableProxy(temp), return_value, pos);
@@ -3181,9 +3181,9 @@ void Parser::InitializeForEachStatement(ForEachStatement* stmt,
   ForOfStatement* for_of = stmt->AsForOfStatement();
 
   if (for_of != NULL) {
-    Variable* iterator = scope_->DeclarationScope()->NewTemporary(
+    Variable* iterator = scope_->NewTemporary(
         ast_value_factory()->dot_iterator_string());
-    Variable* result = scope_->DeclarationScope()->NewTemporary(
+    Variable* result = scope_->NewTemporary(
         ast_value_factory()->dot_result_string());
 
     Expression* assign_iterator;
@@ -3299,7 +3299,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   //   make statement: temp_x = x.
   for (int i = 0; i < names->length(); i++) {
     VariableProxy* proxy = NewUnresolved(names->at(i), LET);
-    Variable* temp = scope_->DeclarationScope()->NewTemporary(temp_name);
+    Variable* temp = scope_->NewTemporary(temp_name);
     VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
     Assignment* assignment = factory()->NewAssignment(
         Token::ASSIGN, temp_proxy, proxy, RelocInfo::kNoPosition);
@@ -3312,7 +3312,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
   Variable* first = NULL;
   // Make statement: first = 1.
   if (next) {
-    first = scope_->DeclarationScope()->NewTemporary(temp_name);
+    first = scope_->NewTemporary(temp_name);
     VariableProxy* first_proxy = factory()->NewVariableProxy(first);
     Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
     Assignment* assignment = factory()->NewAssignment(
@@ -3392,7 +3392,7 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
     ignore_completion_block->AddStatement(clear_first_or_next, zone());
   }
 
-  Variable* flag = scope_->DeclarationScope()->NewTemporary(temp_name);
+  Variable* flag = scope_->NewTemporary(temp_name);
   // Make statement: flag = 1.
   {
     VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
@@ -3578,7 +3578,7 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         //     let x;  // for TDZ
         //   }
 
-        Variable* temp = scope_->DeclarationScope()->NewTemporary(
+        Variable* temp = scope_->NewTemporary(
             ast_value_factory()->dot_for_string());
         ForEachStatement* loop =
             factory()->NewForEachStatement(mode, labels, stmt_pos);
@@ -4007,7 +4007,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
       // Calling a generator returns a generator object.  That object is stored
       // in a temporary variable, a definition that is used by "yield"
       // expressions. This also marks the FunctionState as a generator.
-      Variable* temp = scope_->DeclarationScope()->NewTemporary(
+      Variable* temp = scope_->NewTemporary(
           ast_value_factory()->dot_generator_object_string());
       function_state.set_generator_object_variable(temp);
     }

src/pattern-rewriter.cc

@@ -214,8 +214,8 @@ void Parser::PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
 
 
 Variable* Parser::PatternRewriter::CreateTempVar(Expression* value) {
-  auto temp_scope = descriptor_->parser->scope_->DeclarationScope();
-  auto temp = temp_scope->NewTemporary(ast_value_factory()->empty_string());
+  auto temp = descriptor_->parser->scope_->NewTemporary(
+      ast_value_factory()->empty_string());
   if (value != nullptr) {
     auto assignment = factory()->NewAssignment(
         Token::ASSIGN, factory()->NewVariableProxy(temp), value,

src/runtime/runtime-scopes.cc

@@ -830,7 +830,6 @@ RUNTIME_FUNCTION(Runtime_DeclareModules) {
           USE(result);
           break;
         }
-        case INTERNAL:
         case TEMPORARY:
         case DYNAMIC:
         case DYNAMIC_GLOBAL:

src/scopeinfo.cc

@@ -564,7 +564,7 @@ int ScopeInfo::ContextSlotIndex(Handle<ScopeInfo> scope_info,
     }
     // Cache as not found. Mode, location, init flag and maybe assigned flag
     // don't matter.
-    context_slot_cache->Update(scope_info, name, INTERNAL,
+    context_slot_cache->Update(scope_info, name, TEMPORARY,
                                VariableLocation::CONTEXT, kNeedsInitialization,
                                kNotAssigned, -1);
   }

src/scopes.cc

@@ -527,26 +527,15 @@ void Scope::RemoveUnresolved(VariableProxy* var) {
 }
 
 
-Variable* Scope::NewInternal(const AstRawString* name) {
-  DCHECK(!already_resolved());
-  Variable* var = new(zone()) Variable(this,
-                                       name,
-                                       INTERNAL,
-                                       Variable::NORMAL,
-                                       kCreatedInitialized);
-  internals_.Add(var, zone());
-  return var;
-}
-
-
 Variable* Scope::NewTemporary(const AstRawString* name) {
   DCHECK(!already_resolved());
-  Variable* var = new(zone()) Variable(this,
+  Scope* scope = this->TemporaryScope();
+  Variable* var = new(zone()) Variable(scope,
                                        name,
                                        TEMPORARY,
                                        Variable::NORMAL,
                                        kCreatedInitialized);
-  temps_.Add(var, zone());
+  scope->temps_.Add(var, zone());
   return var;
 }
 
@@ -772,6 +761,15 @@ Scope* Scope::DeclarationScope() {
 }
 
 
+Scope* Scope::TemporaryScope() {
+  Scope* scope = this;
+  while (!scope->is_declaration_scope() || scope->is_block_scope()) {
+    scope = scope->outer_scope();
+  }
+  return scope;
+}
+
+
 Scope* Scope::ReceiverScope() {
   Scope* scope = this;
   while (!scope->is_script_scope() &&
@@ -1354,7 +1352,6 @@ bool Scope::MustAllocateInContext(Variable* var) {
   // always context-allocated.
   if (has_forced_context_allocation()) return true;
   if (var->mode() == TEMPORARY) return false;
-  if (var->mode() == INTERNAL) return true;
   if (is_catch_scope() || is_module_scope()) return true;
   if (is_script_scope() && IsLexicalVariableMode(var->mode())) return true;
   return var->has_forced_context_allocation() ||
@@ -1609,7 +1606,8 @@ void Scope::AllocateModules() {
       DCHECK(!scope->already_resolved());
       DCHECK(scope->module_descriptor_->IsFrozen());
       DCHECK_NULL(scope->module_var_);
-      scope->module_var_ = NewInternal(ast_value_factory_->dot_module_string());
+      scope->module_var_ =
+          NewTemporary(ast_value_factory_->dot_module_string());
       ++num_modules_;
     }
   }

src/scopes.h

@@ -168,16 +168,11 @@ class Scope: public ZoneObject {
   // such a variable again if it was added; otherwise this is a no-op.
   void RemoveUnresolved(VariableProxy* var);
 
-  // Creates a new internal variable in this scope.  The name is only used
-  // for printing and cannot be used to find the variable.  In particular,
-  // the only way to get hold of the temporary is by keeping the Variable*
-  // around.
-  Variable* NewInternal(const AstRawString* name);
-
-  // Creates a new temporary variable in this scope.  The name is only used
-  // for printing and cannot be used to find the variable.  In particular,
-  // the only way to get hold of the temporary is by keeping the Variable*
-  // around.  The name should not clash with a legitimate variable names.
+  // Creates a new temporary variable in this scope's TemporaryScope.  The
+  // name is only used for printing and cannot be used to find the variable.
+  // In particular, the only way to get hold of the temporary is by keeping the
+  // Variable* around.  The name should not clash with a legitimate variable
+  // names.
   Variable* NewTemporary(const AstRawString* name);
 
   // Adds the specific declaration node to the list of declarations in
@@ -487,6 +482,11 @@ class Scope: public ZoneObject {
   // the scope where var declarations will be hoisted to in the implementation.
   Scope* DeclarationScope();
 
+  // Find the first non-block declaration scope. This should be either a script,
+  // function, or eval scope. Same as DeclarationScope(), but skips
+  // declaration "block" scopes. Used for declaring temporaries.
+  Scope* TemporaryScope();
+
   // Find the first (non-arrow) function or script scope.  This is where
   // 'this' is bound, and what determines the function kind.
   Scope* ReceiverScope();

src/variables.cc

@@ -24,7 +24,6 @@ const char* Variable::Mode2String(VariableMode mode) {
     case DYNAMIC: return "DYNAMIC";
     case DYNAMIC_GLOBAL: return "DYNAMIC_GLOBAL";
     case DYNAMIC_LOCAL: return "DYNAMIC_LOCAL";
-    case INTERNAL: return "INTERNAL";
     case TEMPORARY: return "TEMPORARY";
   }
   UNREACHABLE();

test/cctest/test-parsing.cc

@@ -5527,7 +5527,7 @@ TEST(ModuleParsingInternals) {
   CHECK_EQ(1, outer_scope->num_modules());
   CHECK(module_scope->is_module_scope());
   CHECK_NOT_NULL(module_scope->module_var());
-  CHECK_EQ(i::INTERNAL, module_scope->module_var()->mode());
+  CHECK_EQ(i::TEMPORARY, module_scope->module_var()->mode());
   i::ModuleDescriptor* descriptor = module_scope->module();
   CHECK_NOT_NULL(descriptor);
   CHECK_EQ(1, descriptor->Length());

test/mjsunit/es6/regress/regress-cr512574.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
+
+function f({}) {
+  for (var v in []);
+};