From b8ff13111a34553bf17961b9550dcbb7380273f3 Mon Sep 17 00:00:00 2001 From: machenbach Date: Thu, 7 May 2015 00:42:19 -0700 Subject: [PATCH] Revert of Remove Scope::scope_uses_arguments_ flag (patchset #1 id:1 of https://codereview.chromium.org/1124233002/) Reason for revert: [Sheriff] Need to fix compilation after this revert: https://chromium.googlesource.com/v8/v8/+/5cab6be83a44567a3ee5747d728a399025d38411 Original issue's description: > Remove Scope::scope_uses_arguments_ flag > > Use of arguments is tracked as a variable, like any other variable. > > R=arv@chromium.org > LOG=N > BUG= > > Committed: https://crrev.com/d4ea33f480243fb5b7d2cca6edddcaa3e9478e29 > Cr-Commit-Position: refs/heads/master@{#28271} TBR=arv@chromium.org,wingo@igalia.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/1134453002 Cr-Commit-Position: refs/heads/master@{#28284} --- src/preparser.h | 3 +++ src/scopes.cc | 10 ++++++++++ src/scopes.h | 10 ++++++++++ test/cctest/test-parsing.cc | 13 +++++++------ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/preparser.h b/src/preparser.h index 93642812fe..541da23af1 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -2076,6 +2076,7 @@ ParserBase::ParseAndClassifyIdentifier(ExpressionClassifier* classifier, classifier->RecordExpressionError(scanner()->location(), "strong_arguments"); } + if (this->IsArguments(name)) scope_->RecordArgumentsUsage(); return name; } else if (is_sloppy(language_mode()) && (next == Token::FUTURE_STRICT_RESERVED_WORD || @@ -2108,6 +2109,7 @@ typename ParserBase::IdentifierT ParserBase< } IdentifierT name = this->GetSymbol(scanner()); + if (this->IsArguments(name)) scope_->RecordArgumentsUsage(); return name; } @@ -2125,6 +2127,7 @@ ParserBase::ParseIdentifierName(bool* ok) { } IdentifierT name = this->GetSymbol(scanner()); + if (this->IsArguments(name)) scope_->RecordArgumentsUsage(); return name; } diff --git a/src/scopes.cc b/src/scopes.cc index 542a3d3b39..1fcb0fb61d 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -163,6 +163,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope, scope_inside_with_ = false; scope_contains_with_ = false; scope_calls_eval_ = false; + scope_uses_arguments_ = false; scope_uses_super_property_ = false; asm_module_ = false; asm_function_ = outer_scope != NULL && outer_scope->asm_module_; @@ -170,6 +171,7 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope, language_mode_ = outer_scope != NULL ? outer_scope->language_mode_ : SLOPPY; outer_scope_calls_sloppy_eval_ = false; inner_scope_calls_eval_ = false; + inner_scope_uses_arguments_ = false; inner_scope_uses_super_property_ = false; force_eager_compilation_ = false; force_context_allocation_ = (outer_scope != NULL && !is_function_scope()) @@ -374,6 +376,7 @@ Scope* Scope::FinalizeBlockScope() { } // Propagate usage flags to outer scope. + if (uses_arguments()) outer_scope_->RecordArgumentsUsage(); if (uses_super_property()) outer_scope_->RecordSuperPropertyUsage(); if (scope_calls_eval_) outer_scope_->RecordEvalCall(); @@ -921,8 +924,12 @@ void Scope::Print(int n) { if (scope_inside_with_) Indent(n1, "// scope inside 'with'\n"); if (scope_contains_with_) Indent(n1, "// scope contains 'with'\n"); if (scope_calls_eval_) Indent(n1, "// scope calls 'eval'\n"); + if (scope_uses_arguments_) Indent(n1, "// scope uses 'arguments'\n"); if (scope_uses_super_property_) Indent(n1, "// scope uses 'super' property\n"); + if (inner_scope_uses_arguments_) { + Indent(n1, "// inner scope uses 'arguments'\n"); + } if (inner_scope_uses_super_property_) Indent(n1, "// inner scope uses 'super' property\n"); if (outer_scope_calls_sloppy_eval_) { @@ -1273,6 +1280,9 @@ void Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) { // usage of arguments/super/this, but do not propagate them out from normal // functions. if (!inner->is_function_scope() || inner->is_arrow_scope()) { + if (inner->scope_uses_arguments_ || inner->inner_scope_uses_arguments_) { + inner_scope_uses_arguments_ = true; + } if (inner->scope_uses_super_property_ || inner->inner_scope_uses_super_property_) { inner_scope_uses_super_property_ = true; diff --git a/src/scopes.h b/src/scopes.h index b8025cc03b..15fb9d1939 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -211,6 +211,9 @@ class Scope: public ZoneObject { // Inform the scope that the corresponding code contains an eval call. void RecordEvalCall() { if (!is_script_scope()) scope_calls_eval_ = true; } + // Inform the scope that the corresponding code uses "arguments". + void RecordArgumentsUsage() { scope_uses_arguments_ = true; } + // Inform the scope that the corresponding code uses "super". void RecordSuperPropertyUsage() { scope_uses_super_property_ = true; } @@ -307,6 +310,10 @@ class Scope: public ZoneObject { // Does this scope contain a with statement. bool contains_with() const { return scope_contains_with_; } + // Does this scope access "arguments". + bool uses_arguments() const { return scope_uses_arguments_; } + // Does any inner scope access "arguments". + bool inner_uses_arguments() const { return inner_scope_uses_arguments_; } // Does this scope access "super" property (super.foo). bool uses_super_property() const { return scope_uses_super_property_; } // Does any inner scope access "super" property. @@ -559,6 +566,8 @@ class Scope: public ZoneObject { // This scope or a nested catch scope or with scope contain an 'eval' call. At // the 'eval' call site this scope is the declaration scope. bool scope_calls_eval_; + // This scope uses "arguments". + bool scope_uses_arguments_; // This scope uses "super" property ('super.foo'). bool scope_uses_super_property_; // This scope contains an "use asm" annotation. @@ -574,6 +583,7 @@ class Scope: public ZoneObject { // Computed via PropagateScopeInfo. bool outer_scope_calls_sloppy_eval_; bool inner_scope_calls_eval_; + bool inner_scope_uses_arguments_; bool inner_scope_uses_super_property_; bool force_eager_compilation_; bool force_context_allocation_; diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 92be55b91b..f0abbc958f 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -960,6 +960,7 @@ TEST(ScopeUsesArgumentsSuperThis) { ARGUMENTS = 1, SUPER_PROPERTY = 1 << 1, THIS = 1 << 2, + INNER_ARGUMENTS = 1 << 3, INNER_SUPER_PROPERTY = 1 << 4, }; @@ -1000,9 +1001,9 @@ TEST(ScopeUsesArgumentsSuperThis) { {"return { m(x) { return () => super.m() } }", NONE}, // Flags must be correctly set when using block scoping. {"\"use strict\"; while (true) { let x; this, arguments; }", - ARGUMENTS | THIS}, + INNER_ARGUMENTS | THIS}, {"\"use strict\"; while (true) { let x; this, super.f(), arguments; }", - ARGUMENTS | INNER_SUPER_PROPERTY | THIS}, + INNER_ARGUMENTS | INNER_SUPER_PROPERTY | THIS}, {"\"use strict\"; if (foo()) { let x; this.f() }", THIS}, {"\"use strict\"; if (foo()) { let x; super.f() }", INNER_SUPER_PROPERTY}, @@ -1063,10 +1064,8 @@ TEST(ScopeUsesArgumentsSuperThis) { CHECK_EQ(1, scope->inner_scopes()->length()); scope = scope->inner_scopes()->at(0); } - // Arguments usage in an arrow function doesn't cause local allocation of - // an arguments object. - CHECK_EQ((source_data[i].expected & ARGUMENTS) != 0 && j != 1, - scope->arguments() != nullptr); + CHECK_EQ((source_data[i].expected & ARGUMENTS) != 0, + scope->uses_arguments()); CHECK_EQ((source_data[i].expected & SUPER_PROPERTY) != 0, scope->uses_super_property()); if ((source_data[i].expected & THIS) != 0) { @@ -1074,6 +1073,8 @@ TEST(ScopeUsesArgumentsSuperThis) { // script scope are marked as used. CHECK(scope->LookupThis()->is_used()); } + CHECK_EQ((source_data[i].expected & INNER_ARGUMENTS) != 0, + scope->inner_uses_arguments()); CHECK_EQ((source_data[i].expected & INNER_SUPER_PROPERTY) != 0, scope->inner_uses_super_property()); }