[ignition] Add a DCHECK that scopes are visited in nesting order

Also fix one apparently-harmless bug in the Parser where we failed
to Finalize an empty scope. Without this fix, the DCHECK fails
on any C-style for-loop with a lexical binding that hits the
fast path (this is well-covered by many existing tests in mjsunit).

Thanks to Georg Neis for the suggestion of this DCHECK.

Change-Id: Ie1a8f8809f4d152c87f2da08209c610514645827
Reviewed-on: https://chromium-review.googlesource.com/587750
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47046}
This commit is contained in:
Adam Klein 2017-07-31 11:20:06 -07:00 committed by Commit Bot
parent e11332f34b
commit 5ff10f2060
4 changed files with 18 additions and 14 deletions

View File

@ -731,6 +731,7 @@ class BytecodeGenerator::CurrentScope final {
CurrentScope(BytecodeGenerator* generator, Scope* scope)
: generator_(generator), outer_scope_(generator->current_scope()) {
if (scope != nullptr) {
DCHECK_EQ(outer_scope_, scope->outer_scope());
generator_->set_current_scope(scope);
}
}

View File

@ -5751,12 +5751,16 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStandardForLoop(
scope()->set_end_position(scanner()->location().end_pos);
inner_scope->set_end_position(scanner()->location().end_pos);
if (bound_names_are_lexical && for_info->bound_names.length() > 0 &&
function_state_->contains_function_or_eval()) {
scope()->set_is_hidden();
return impl()->DesugarLexicalBindingsInForStatement(
loop, init, cond, next, body, body_range, inner_scope, *for_info,
CHECK_OK);
if (bound_names_are_lexical && for_info->bound_names.length() > 0) {
if (function_state_->contains_function_or_eval()) {
scope()->set_is_hidden();
return impl()->DesugarLexicalBindingsInForStatement(
loop, init, cond, next, body, body_range, inner_scope, *for_info,
CHECK_OK);
} else {
inner_scope = inner_scope->FinalizeBlockScope();
CHECK_NULL(inner_scope);
}
}
Scope* for_scope = scope()->FinalizeBlockScope();

View File

@ -3603,9 +3603,9 @@ TEST(MaybeAssignedInsideLoop) {
{1, "for (let j=x; j<10; ++j) { [foo] = [j] }", top},
{1, "for (let j=x; j<10; ++j) { var foo = j }", top},
{1, "for (let j=x; j<10; ++j) { var [foo] = [j] }", top},
{0, "for (let j=x; j<10; ++j) { let foo = j }", {0, 0, 0}},
{0, "for (let j=x; j<10; ++j) { let foo = j }", {0, 0}},
{0, "for (let j=x; j<10; ++j) { let [foo] = [j] }", {0, 0, 0}},
{0, "for (let j=x; j<10; ++j) { const foo = j }", {0, 0, 0}},
{0, "for (let j=x; j<10; ++j) { const foo = j }", {0, 0}},
{0, "for (let j=x; j<10; ++j) { const [foo] = [j] }", {0, 0, 0}},
{0, "for (let j=x; j<10; ++j) { function foo() {return j} }", {0, 0, 0}},
@ -3613,9 +3613,9 @@ TEST(MaybeAssignedInsideLoop) {
{1, "for (let {j}=x; j<10; ++j) { [foo] = [j] }", top},
{1, "for (let {j}=x; j<10; ++j) { var foo = j }", top},
{1, "for (let {j}=x; j<10; ++j) { var [foo] = [j] }", top},
{0, "for (let {j}=x; j<10; ++j) { let foo = j }", {0, 0, 0}},
{0, "for (let {j}=x; j<10; ++j) { let foo = j }", {0, 0}},
{0, "for (let {j}=x; j<10; ++j) { let [foo] = [j] }", {0, 0, 0}},
{0, "for (let {j}=x; j<10; ++j) { const foo = j }", {0, 0, 0}},
{0, "for (let {j}=x; j<10; ++j) { const foo = j }", {0, 0}},
{0, "for (let {j}=x; j<10; ++j) { const [foo] = [j] }", {0, 0, 0}},
{0, "for (let {j}=x; j<10; ++j) { function foo(){return j} }", {0, 0, 0}},
@ -10312,7 +10312,7 @@ TEST(LexicalLoopVariable) {
CHECK_NOT_NULL(loop_var);
CHECK(loop_var->IsStackLocal());
CHECK_EQ(loop_block->ContextLocalCount(), 0);
CHECK_EQ(loop_block->inner_scope()->ContextLocalCount(), 0);
CHECK_NULL(loop_block->inner_scope());
});
}

View File

@ -276,14 +276,13 @@ function *gen10() {
g = gen10();
g.next();
CheckScopeChain([debug.ScopeType.Block,
debug.ScopeType.Block,
debug.ScopeType.Local,
debug.ScopeType.Script,
debug.ScopeType.Global], g);
CheckScopeContent({i: 0}, 1, g);
CheckScopeContent({i: 0}, 0, g);
g.next();
CheckScopeContent({i: 1}, 1, g);
CheckScopeContent({i: 1}, 0, g);
// Nested generators.