Reviewers: Michael Starzinger,
Description:
[es6] Make sure temporaries are not allocated in block scope
While at it, remove the notion of INTERNAL variables.
[email protected]
BUG=cr:512574
Please review this at https://codereview.chromium.org/1250513004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+39, -47 lines):
M src/contexts.cc
M src/globals.h
M src/parser.cc
M src/pattern-rewriter.cc
M src/runtime/runtime-scopes.cc
M src/scopeinfo.cc
M src/scopes.h
M src/scopes.cc
M src/variables.cc
M test/cctest/test-parsing.cc
A + test/mjsunit/es6/regress/regress-cr512574.js
Index: src/contexts.cc
diff --git a/src/contexts.cc b/src/contexts.cc
index
b29405b505b2316236fad7168366207d54bfbd92..88f593fab8525a197cc2d13d47dbdf8ece4a3292
100644
--- a/src/contexts.cc
+++ b/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;
Index: src/globals.h
diff --git a/src/globals.h b/src/globals.h
index
749d6e1c3beb8a06a15c656021d8b14f8f4d22bc..7ab6fdbd5f5cced8ce0b66a40d267950f6b9f2fb
100644
--- a/src/globals.h
+++ b/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
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
60b72282c1f454d2d74a0864fe599cc0ed1ae72b..4644452b3fd42c0baf5ce0956a16b1e07660c0f3
100644
--- a/src/parser.cc
+++ b/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);
}
Index: src/pattern-rewriter.cc
diff --git a/src/pattern-rewriter.cc b/src/pattern-rewriter.cc
index
6969cf214ed70034fce8f58a737724ab4b600700..10702d65cee6a49c1db4aa0029cff66a868d496e
100644
--- a/src/pattern-rewriter.cc
+++ b/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,
Index: src/runtime/runtime-scopes.cc
diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc
index
b4506512a188e92ee9a02e341582a7fbe1ce30f6..948f7295537378fbc886d9b7bf86e40b18983b66
100644
--- a/src/runtime/runtime-scopes.cc
+++ b/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:
Index: src/scopeinfo.cc
diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index
4ed22b19dcd9a37b60f07aacd92b09c923079ca2..a621083f9679afe6dc9ca61b5b8f22322a8ce33f
100644
--- a/src/scopeinfo.cc
+++ b/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);
}
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
6e1c18dc2e11b346c3c3e4914d58160a8027eb5c..4aca42042233652d3ad0b597804793652e8b106d
100644
--- a/src/scopes.cc
+++ b/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_;
}
}
Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index
e565ff69fe25f7c1197a341a5e96940ced953773..73277252e3ed1bb451f8507603256358ab03ac29
100644
--- a/src/scopes.h
+++ b/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();
Index: src/variables.cc
diff --git a/src/variables.cc b/src/variables.cc
index
b8fddd306df59ff86b15d00a7e7df009e4662261..5833d860aab2af36b36c60296b9447689bb0014e
100644
--- a/src/variables.cc
+++ b/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();
Index: test/cctest/test-parsing.cc
diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc
index
e14b6f63601708ada88f950d1cc53db51e029e08..703f74d947cdaba3fb1da8f9cea83a0c0c6226ac
100644
--- a/test/cctest/test-parsing.cc
+++ b/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());
Index: test/mjsunit/es6/regress/regress-cr512574.js
diff --git a/test/message/destructuring-modify-const.js
b/test/mjsunit/es6/regress/regress-cr512574.js
similarity index 76%
copy from test/message/destructuring-modify-const.js
copy to test/mjsunit/es6/regress/regress-cr512574.js
index
cabd924b37ea9f178259f4c979c3b3268920c713..2c10e193156550e1f23d35a070027054ba652eb8
100644
--- a/test/message/destructuring-modify-const.js
+++ b/test/mjsunit/es6/regress/regress-cr512574.js
@@ -3,7 +3,7 @@
// found in the LICENSE file.
// Flags: --harmony-destructuring
-'use strict';
-const { x : x, y : y } = { x : 1, y : 2 };
-x++;
+function f({}) {
+ for (var v in []);
+};
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
