Commit 62572e01 authored by adamk's avatar adamk Committed by Commit bot

[es6] Ensure that for-in/of loops have a proper TDZ for their lexically-bound variables

The enumerable expression in a for-in/of loop is supposed to have a TDZ for any
lexically bound names in that loop (there can be more than one with destructuring).

This patch accomplishes this with an almost-correct desugaring. The only thing missing
is proper debugger support (the let declarations added by the desugaring, while invisible
to code due to shadowing, are visible to the debugger).

BUG=v8:4210
LOG=n

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

Cr-Commit-Position: refs/heads/master@{#29396}
parent 9adb5f0a
......@@ -3555,11 +3555,14 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
//
// into
//
// <let x' be a temporary variable>
// for (x' in/of e) {
// let/const/var x;
// x = x';
// b;
// {
// <let x' be a temporary variable>
// for (x' in/of e) {
// let/const/var x;
// x = x';
// b;
// }
// let x; // for TDZ
// }
Variable* temp = scope_->DeclarationScope()->NewTemporary(
......@@ -3568,12 +3571,14 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
factory()->NewForEachStatement(mode, labels, stmt_pos);
Target target(&this->target_stack_, loop);
// The expression does not see the lexical loop variables.
scope_ = saved_scope;
Expression* enumerable = ParseExpression(true, CHECK_OK);
scope_ = for_scope;
Expect(Token::RPAREN, CHECK_OK);
Scope* body_scope = NewScope(scope_, BLOCK_SCOPE);
body_scope->set_start_position(scanner()->location().beg_pos);
scope_ = body_scope;
Statement* body = ParseSubStatement(NULL, CHECK_OK);
Block* body_block =
......@@ -3601,17 +3606,46 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
VariableProxy* temp_proxy =
factory()->NewVariableProxy(temp, each_beg_pos, each_end_pos);
InitializeForEachStatement(loop, temp_proxy, enumerable, body_block);
scope_ = for_scope;
body_scope->set_end_position(scanner()->location().end_pos);
body_scope = body_scope->FinalizeBlockScope();
if (body_scope != nullptr) {
body_block->set_scope(body_scope);
}
// Create a TDZ for any lexically-bound names.
if (is_strict(language_mode()) &&
IsLexicalVariableMode(parsing_result.descriptor.mode)) {
DCHECK_NULL(init_block);
init_block =
factory()->NewBlock(nullptr, 1, false, RelocInfo::kNoPosition);
for (int i = 0; i < lexical_bindings.length(); ++i) {
// TODO(adamk): This needs to be some sort of special
// INTERNAL variable that's invisible to the debugger
// but visible to everything else.
VariableProxy* tdz_proxy = NewUnresolved(lexical_bindings[i], LET);
Declaration* tdz_decl = factory()->NewVariableDeclaration(
tdz_proxy, LET, scope_, RelocInfo::kNoPosition);
Variable* tdz_var = Declare(tdz_decl, DeclarationDescriptor::NORMAL,
true, CHECK_OK);
tdz_var->set_initializer_position(position());
}
}
scope_ = saved_scope;
for_scope->set_end_position(scanner()->location().end_pos);
for_scope = for_scope->FinalizeBlockScope();
if (for_scope != nullptr) {
body_block->set_scope(for_scope);
}
// Parsed for-in loop w/ variable declarations.
if (init_block != nullptr) {
init_block->AddStatement(loop, zone());
if (for_scope != nullptr) {
init_block->set_scope(for_scope);
}
return init_block;
} else {
DCHECK_NULL(for_scope);
return loop;
}
} else {
......
......@@ -372,13 +372,16 @@ function for_loop_1() {
listener_delegate = function(exec_state) {
CheckScopeChain([debug.ScopeType.Block,
debug.ScopeType.Block,
debug.ScopeType.Local,
debug.ScopeType.Script,
debug.ScopeType.Global], exec_state);
CheckScopeContent({x:'y'}, 0, exec_state);
// The function scope contains a temporary iteration variable, but it is
// hidden to the debugger.
CheckScopeContent({}, 1, exec_state);
// TODO(adamk): This variable is only used to provide a TDZ for the enumerable
// expression and should not be visible to the debugger.
CheckScopeContent({x:undefined}, 1, exec_state);
};
for_loop_1();
EndTest();
......@@ -397,6 +400,7 @@ function for_loop_2() {
listener_delegate = function(exec_state) {
CheckScopeChain([debug.ScopeType.Block,
debug.ScopeType.Block,
debug.ScopeType.Block,
debug.ScopeType.Local,
debug.ScopeType.Script,
......@@ -405,7 +409,9 @@ listener_delegate = function(exec_state) {
CheckScopeContent({x:'y'}, 1, exec_state);
// The function scope contains a temporary iteration variable, hidden to the
// debugger.
CheckScopeContent({}, 2, exec_state);
// TODO(adamk): This variable is only used to provide a TDZ for the enumerable
// expression and should not be visible to the debugger.
CheckScopeContent({x:undefined}, 2, exec_state);
};
for_loop_2();
EndTest();
......
......@@ -110,7 +110,7 @@ var expected = [
// Exit.
"y0","z0",
]
print("expected:\n"+ JSON.stringify(log));
print("expected:\n"+ JSON.stringify(expected));
assertArrayEquals(expected, log);
assertEquals(48, s);
......
......@@ -18,15 +18,15 @@ assertEquals(3, f[2]());
let x = 1;
s = 0;
for (const x of [x, x+1, x+2]) {
s += x;
for (const z of [x, x+1, x+2]) {
s += z;
}
assertEquals(6, s);
s = 0;
var q = 1;
for (const q of [q, q+1, q+2]) {
s += q;
for (const x of [q, q+1, q+2]) {
s += x;
}
assertEquals(6, s);
......@@ -56,15 +56,15 @@ assertThrows("'use strict'; for (const x in [1,2,3]) { x++ }", TypeError);
let x = 1;
s = 0;
for (const x of [x, x+1, x+2]) {
s += x;
for (const q of [x, x+1, x+2]) {
s += q;
}
assertEquals(6, s);
s = 0;
var q = 1;
for (const q of [q, q+1, q+2]) {
s += q;
for (const x of [q, q+1, q+2]) {
s += x;
}
assertEquals(6, s);
......
......@@ -728,3 +728,11 @@
assertThrows("function f({x}) { var x; }; f({});", SyntaxError);
assertThrows("'use strict'; function f({x}) { let x = 0; }; f({});", SyntaxError);
}());
(function TestForInOfTDZ() {
assertThrows("'use strict'; let x = {}; for (let [x, y] of {x});", ReferenceError);
assertThrows("'use strict'; let x = {}; for (let [y, x] of {x});", ReferenceError);
assertThrows("'use strict'; let x = {}; for (let [x, y] in {x});", ReferenceError);
assertThrows("'use strict'; let x = {}; for (let [y, x] in {x});", ReferenceError);
}());
......@@ -193,10 +193,6 @@ function assertThrowsHelper(code) {
i;
}
let var6 = [1, 2];
// The second var6 resolves to outside (not to the first var6).
for (let var6 of var6) { var6; }
try {
throw "error";
} catch (e) {
......
......@@ -464,12 +464,6 @@
# https://code.google.com/p/v8/issues/detail?id=3673
'language/statements/class/definition/basics': [FAIL],
# https://code.google.com/p/v8/issues/detail?id=4210
'language/statements/for-in/const-bound-names-fordecl-tdz-for-in': [PASS, FAIL],
'language/statements/for-in/let-bound-names-fordecl-tdz-for-in': [PASS, FAIL],
'language/statements/for-of/const-bound-names-fordecl-tdz-for-of': [PASS, FAIL],
'language/statements/for-of/let-bound-names-fordecl-tdz-for-of': [PASS, FAIL],
# Destructuring
# https://code.google.com/p/v8/issues/detail?id=811
'language/statements/for-of/body-dstr-assign': [FAIL],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment