Commit b04d1d0e authored by marja's avatar marja Committed by Commit bot

[parser] Skipping inner funcs: produce the same scopes / variables for (some) loops.

Turns out is_hidden is not the right condition for "scope should be present in
the preparse data". For now, replaced it with "is hidden leaf scope" (i.e.,
doesn't contain any non-hidden scopes). That's probably not the right condition
either; will be fixed once there's more data to decide what the right condition
is.

BUG=v8:5516
R=vogelheim@chromium.org

Review-Url: https://codereview.chromium.org/2669163002
Cr-Commit-Position: refs/heads/master@{#42909}
parent 45721b71
...@@ -5660,6 +5660,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStandardForLoop( ...@@ -5660,6 +5660,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseStandardForLoop(
auto result = impl()->DesugarLexicalBindingsInForStatement( auto result = impl()->DesugarLexicalBindingsInForStatement(
loop, init, cond, next, body, inner_scope, *for_info, CHECK_OK); loop, init, cond, next, body, inner_scope, *for_info, CHECK_OK);
for_state->set_end_position(scanner()->location().end_pos); for_state->set_end_position(scanner()->location().end_pos);
inner_scope->set_end_position(scanner()->location().end_pos);
return result; return result;
} }
......
...@@ -2383,7 +2383,6 @@ Statement* Parser::DesugarLexicalBindingsInForStatement( ...@@ -2383,7 +2383,6 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
inner_block->statements()->Add(ignore_completion_block, zone()); inner_block->statements()->Add(ignore_completion_block, zone());
} }
inner_scope->set_end_position(scanner()->location().end_pos);
inner_block->set_scope(inner_scope); inner_block->set_scope(inner_scope);
} }
......
...@@ -311,6 +311,9 @@ void PreParser::DeclareAndInitializeVariables( ...@@ -311,6 +311,9 @@ void PreParser::DeclareAndInitializeVariables(
declaration_descriptor->scope->RemoveUnresolved(variable); declaration_descriptor->scope->RemoveUnresolved(variable);
scope()->DeclareVariableName(variable->raw_name(), scope()->DeclareVariableName(variable->raw_name(),
declaration_descriptor->mode); declaration_descriptor->mode);
if (names) {
names->Add(variable->raw_name(), zone());
}
} }
} }
} }
......
...@@ -1336,6 +1336,11 @@ class PreParser : public ParserBase<PreParser> { ...@@ -1336,6 +1336,11 @@ class PreParser : public ParserBase<PreParser> {
PreParserExpression cond, PreParserStatement next, PreParserExpression cond, PreParserStatement next,
PreParserStatement body, Scope* inner_scope, const ForInfo& for_info, PreParserStatement body, Scope* inner_scope, const ForInfo& for_info,
bool* ok) { bool* ok) {
// See Parser::DesugarLexicalBindingsInForStatement.
for (int i = 0; i < for_info.bound_names.length(); i++) {
inner_scope->DeclareVariableName(for_info.bound_names[i],
for_info.parsing_result.descriptor.mode);
}
return loop; return loop;
} }
......
...@@ -8835,6 +8835,20 @@ class ScopeTestHelper { ...@@ -8835,6 +8835,20 @@ class ScopeTestHelper {
return var->scope()->MustAllocateInContext(var); return var->scope()->MustAllocateInContext(var);
} }
// True if the scope is and its entire subscope tree are hidden.
static bool ScopeTreeIsHidden(Scope* scope) {
if (!scope->is_hidden()) {
return false;
}
for (Scope* inner = scope->inner_scope(); inner != nullptr;
inner = inner->sibling()) {
if (!ScopeTreeIsHidden(inner)) {
return false;
}
}
return true;
}
static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data, static void CompareScopeToData(Scope* scope, const PreParsedScopeData* data,
size_t& index) { size_t& index) {
CHECK_EQ(data->backing_store_[index++], scope->scope_type()); CHECK_EQ(data->backing_store_[index++], scope->scope_type());
...@@ -8844,7 +8858,9 @@ class ScopeTestHelper { ...@@ -8844,7 +8858,9 @@ class ScopeTestHelper {
int inner_scope_count = 0; int inner_scope_count = 0;
for (Scope* inner = scope->inner_scope(); inner != nullptr; for (Scope* inner = scope->inner_scope(); inner != nullptr;
inner = inner->sibling()) { inner = inner->sibling()) {
if (!inner->is_hidden()) { // FIXME(marja): This is probably not the right condition for knowing what
// scopes are present in the preparse data.
if (!ScopeTreeIsHidden(inner)) {
++inner_scope_count; ++inner_scope_count;
} }
} }
...@@ -8878,7 +8894,7 @@ class ScopeTestHelper { ...@@ -8878,7 +8894,7 @@ class ScopeTestHelper {
for (Scope* inner = scope->inner_scope(); inner != nullptr; for (Scope* inner = scope->inner_scope(); inner != nullptr;
inner = inner->sibling()) { inner = inner->sibling()) {
if (!inner->is_hidden()) { if (!ScopeTreeIsHidden(inner)) {
CompareScopeToData(inner, data, index); CompareScopeToData(inner, data, index);
} }
} }
...@@ -9311,6 +9327,22 @@ TEST(PreParserScopeAnalysis) { ...@@ -9311,6 +9327,22 @@ TEST(PreParserScopeAnalysis) {
{"", "let var1; function f1() { eval(''); }"}, {"", "let var1; function f1() { eval(''); }"},
{"", "const var1 = 10; eval('');"}, {"", "const var1 = 10; eval('');"},
{"", "const var1 = 10; function f1() { eval(''); }"}, {"", "const var1 = 10; function f1() { eval(''); }"},
{"", "for (var var1 = 0; var1 < 10; ++var1) { }"},
{"", "for (let var1 = 0; var1 < 10; ++var1) { }"},
{"", "for (const var1 = 0; var1 < 10; ++var1) { }"},
// FIXME(marja): make the corresponding cases work when foo is a sloppy
// block function.
{"",
"'use strict'; for (var var1 = 0; var1 < 10; ++var1) { function foo() { "
"var1; } }"},
{"",
"'use strict'; for (let var1 = 0; var1 < 10; ++var1) { function foo() { "
"var1; } }"},
{"",
"'use strict'; for (const var1 = 0; var1 < 10; ++var1) { function foo() "
"{ var1; } }"},
}; };
for (unsigned i = 0; i < arraysize(inners); ++i) { for (unsigned i = 0; i < arraysize(inners); ++i) {
......
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