From 0841f7241b2c36904f781b71859bf3f77a854af5 Mon Sep 17 00:00:00 2001 From: "wingo@igalia.com" Date: Thu, 16 Oct 2014 13:19:36 +0000 Subject: [PATCH] Track usage of "this" and "arguments" in Scope This adds flags in Scope to track wheter a Scope uses "this" and, "arguments". The information is exposed via Scope::uses_this(), and Scope::uses_arguments(), respectively. Flags for tracking usage on any inner scope uses are available as well via Scope::inner_uses_this(), and Scope::inner_uses_arguments(). Knowing whether scopes use "this" and "arguments" will be handy to generate the code needed to capture their values when generating the code for arrow functions. BUG=v8:2700 LOG= R=rossberg@chromium.org Review URL: https://codereview.chromium.org/422923004 Patch from Adrian Perez de Castro . git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@24663 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/ast-value-factory.h | 6 +++ src/globals.h | 3 +- src/objects.h | 14 ++--- src/preparser.h | 25 ++++++--- src/runtime/runtime-debug.cc | 7 ++- src/scopeinfo.cc | 10 ++-- src/scopes.cc | 22 ++++++++ src/scopes.h | 26 +++++++++- test/cctest/test-parsing.cc | 121 +++++++++++++++++++++++++++++++++++++++++-- 9 files changed, 209 insertions(+), 25 deletions(-) diff --git a/src/ast-value-factory.h b/src/ast-value-factory.h index 82e6e6b..09702eb 100644 --- a/src/ast-value-factory.h +++ b/src/ast-value-factory.h @@ -88,6 +88,8 @@ class AstRawString : public AstString { return *c; } + V8_INLINE bool IsArguments(AstValueFactory* ast_value_factory) const; + // For storing AstRawStrings in a hash map. uint32_t hash() const { return hash_; @@ -340,6 +342,10 @@ class AstValueFactory { #undef F }; + +bool AstRawString::IsArguments(AstValueFactory* ast_value_factory) const { + return ast_value_factory->arguments_string() == this; +} } } // namespace v8::internal #undef STRING_CONSTANTS diff --git a/src/globals.h b/src/globals.h index bef22cf..0e6fcae 100644 --- a/src/globals.h +++ b/src/globals.h @@ -648,7 +648,8 @@ enum ScopeType { GLOBAL_SCOPE, // The top-level scope for a program or a top-level eval. CATCH_SCOPE, // The scope introduced by catch. BLOCK_SCOPE, // The scope introduced by a new block. - WITH_SCOPE // The scope introduced by with. + WITH_SCOPE, // The scope introduced by with. + ARROW_SCOPE // The top-level scope for an arrow function literal. }; diff --git a/src/objects.h b/src/objects.h index 0e4247e..37ab88d 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4312,13 +4312,13 @@ class ScopeInfo : public FixedArray { }; // Properties of scopes. - class ScopeTypeField: public BitField {}; - class CallsEvalField: public BitField {}; - class StrictModeField: public BitField {}; - class FunctionVariableField: public BitField {}; - class FunctionVariableMode: public BitField {}; - class AsmModuleField : public BitField {}; - class AsmFunctionField : public BitField {}; + class ScopeTypeField : public BitField {}; + class CallsEvalField : public BitField {}; + class StrictModeField : public BitField {}; + class FunctionVariableField : public BitField {}; + class FunctionVariableMode : public BitField {}; + class AsmModuleField : public BitField {}; + class AsmFunctionField : public BitField {}; // BitFields representing the encoded information for context locals in the // ContextLocalInfoEntries part. diff --git a/src/preparser.h b/src/preparser.h index a41b228..20bf9e9 100644 --- a/src/preparser.h +++ b/src/preparser.h @@ -615,7 +615,9 @@ class PreParserIdentifier { return PreParserIdentifier(kConstructorIdentifier); } bool IsEval() const { return type_ == kEvalIdentifier; } - bool IsArguments() const { return type_ == kArgumentsIdentifier; } + bool IsArguments(const AstValueFactory* = NULL) const { + return type_ == kArgumentsIdentifier; + } bool IsYield() const { return type_ == kYieldIdentifier; } bool IsPrototype() const { return type_ == kPrototypeIdentifier; } bool IsConstructor() const { return type_ == kConstructorIdentifier; } @@ -956,6 +958,8 @@ class PreParserScope { bool IsDeclared(const PreParserIdentifier& identifier) const { return false; } void DeclareParameter(const PreParserIdentifier& identifier, VariableMode) {} + void RecordArgumentsUsage() {} + void RecordThisUsage() {} // Allow scope->Foo() to work. PreParserScope* operator->() { return this; } @@ -1615,6 +1619,8 @@ typename ParserBase::IdentifierT ParserBase::ParseIdentifier( ReportMessage("strict_eval_arguments"); *ok = false; } + if (name->IsArguments(this->ast_value_factory())) + scope_->RecordArgumentsUsage(); return name; } else if (strict_mode() == SLOPPY && (next == Token::FUTURE_STRICT_RESERVED_WORD || @@ -1645,7 +1651,11 @@ typename ParserBase::IdentifierT ParserBase< *ok = false; return Traits::EmptyIdentifier(); } - return this->GetSymbol(scanner()); + + IdentifierT name = this->GetSymbol(scanner()); + if (name->IsArguments(this->ast_value_factory())) + scope_->RecordArgumentsUsage(); + return name; } @@ -1660,7 +1670,11 @@ ParserBase::ParseIdentifierName(bool* ok) { *ok = false; return Traits::EmptyIdentifier(); } - return this->GetSymbol(scanner()); + + IdentifierT name = this->GetSymbol(scanner()); + if (name->IsArguments(this->ast_value_factory())) + scope_->RecordArgumentsUsage(); + return name; } @@ -1738,6 +1752,7 @@ ParserBase::ParsePrimaryExpression(bool* ok) { switch (token) { case Token::THIS: { Consume(Token::THIS); + scope_->RecordThisUsage(); result = this->ThisExpression(scope_, factory()); break; } @@ -2591,9 +2606,7 @@ template typename ParserBase::ExpressionT ParserBase< Traits>::ParseArrowFunctionLiteral(int start_pos, ExpressionT params_ast, bool* ok) { - // TODO(aperez): Change this to use ARROW_SCOPE - typename Traits::Type::ScopePtr scope = - this->NewScope(scope_, FUNCTION_SCOPE); + typename Traits::Type::ScopePtr scope = this->NewScope(scope_, ARROW_SCOPE); typename Traits::Type::StatementList body; typename Traits::Type::AstProperties ast_properties; BailoutReason dont_optimize_reason = kNoReason; diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index a85820c..af0e574 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -1132,7 +1132,8 @@ class ScopeIterator { context_ = Handle(context_->previous(), isolate_); } } - if (scope_info->scope_type() == FUNCTION_SCOPE) { + if (scope_info->scope_type() == FUNCTION_SCOPE || + scope_info->scope_type() == ARROW_SCOPE) { nested_scope_chain_.Add(scope_info); } } else { @@ -1142,7 +1143,8 @@ class ScopeIterator { // Check whether we are in global, eval or function code. Handle scope_info(shared_info->scope_info()); - if (scope_info->scope_type() != FUNCTION_SCOPE) { + if (scope_info->scope_type() != FUNCTION_SCOPE && + scope_info->scope_type() != ARROW_SCOPE) { // Global or eval code. CompilationInfoWithZone info(script); if (scope_info->scope_type() == GLOBAL_SCOPE) { @@ -1215,6 +1217,7 @@ class ScopeIterator { Handle scope_info = nested_scope_chain_.last(); switch (scope_info->scope_type()) { case FUNCTION_SCOPE: + case ARROW_SCOPE: DCHECK(context_->IsFunctionContext() || !scope_info->HasContext()); return ScopeTypeLocal; case MODULE_SCOPE: diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc index 75bf014..598c5e6 100644 --- a/src/scopeinfo.cc +++ b/src/scopeinfo.cc @@ -170,11 +170,11 @@ int ScopeInfo::ContextLength() { int context_locals = ContextLocalCount(); bool function_name_context_slot = FunctionVariableField::decode(Flags()) == CONTEXT; - bool has_context = context_locals > 0 || - function_name_context_slot || - scope_type() == WITH_SCOPE || - (scope_type() == FUNCTION_SCOPE && CallsEval()) || - scope_type() == MODULE_SCOPE; + bool has_context = context_locals > 0 || function_name_context_slot || + scope_type() == WITH_SCOPE || + (scope_type() == ARROW_SCOPE && CallsEval()) || + (scope_type() == FUNCTION_SCOPE && CallsEval()) || + scope_type() == MODULE_SCOPE; if (has_context) { return Context::MIN_CONTEXT_SLOTS + context_locals + (function_name_context_slot ? 1 : 0); diff --git a/src/scopes.cc b/src/scopes.cc index de364a5..fd803b3 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -160,12 +160,16 @@ void Scope::SetDefaults(ScopeType scope_type, scope_inside_with_ = false; scope_contains_with_ = false; scope_calls_eval_ = false; + scope_uses_this_ = false; + scope_uses_arguments_ = false; asm_module_ = false; asm_function_ = outer_scope != NULL && outer_scope->asm_module_; // Inherit the strict mode from the parent scope. strict_mode_ = outer_scope != NULL ? outer_scope->strict_mode_ : SLOPPY; outer_scope_calls_sloppy_eval_ = false; inner_scope_calls_eval_ = false; + inner_scope_uses_this_ = false; + inner_scope_uses_arguments_ = false; force_eager_compilation_ = false; force_context_allocation_ = (outer_scope != NULL && !is_function_scope()) ? outer_scope->has_forced_context_allocation() : false; @@ -779,6 +783,7 @@ static const char* Header(ScopeType scope_type) { case CATCH_SCOPE: return "catch"; case BLOCK_SCOPE: return "block"; case WITH_SCOPE: return "with"; + case ARROW_SCOPE: return "arrow"; } UNREACHABLE(); return NULL; @@ -885,6 +890,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_this_) Indent(n1, "// scope uses 'this'\n"); + if (scope_uses_arguments_) Indent(n1, "// scope uses 'arguments'\n"); + if (inner_scope_uses_this_) Indent(n1, "// inner scope uses 'this'\n"); + if (inner_scope_uses_arguments_) { + Indent(n1, "// inner scope uses 'arguments'\n"); + } if (outer_scope_calls_sloppy_eval_) { Indent(n1, "// outer scope calls 'eval' in sloppy context\n"); } @@ -1166,6 +1177,17 @@ void Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) { if (inner->scope_calls_eval_ || inner->inner_scope_calls_eval_) { inner_scope_calls_eval_ = true; } + // If the inner scope is an arrow function, propagate the flags tracking + // usage of this/arguments, but do not propagate them out from normal + // functions. + if (!inner->is_function_scope() || inner->is_arrow_scope()) { + if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) { + inner_scope_uses_this_ = true; + } + if (inner->scope_uses_arguments_ || inner->inner_scope_uses_arguments_) { + inner_scope_uses_arguments_ = true; + } + } if (inner->force_eager_compilation_) { force_eager_compilation_ = true; } diff --git a/src/scopes.h b/src/scopes.h index 06c6c99..25305de 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -211,6 +211,12 @@ class Scope: public ZoneObject { // Inform the scope that the corresponding code contains an eval call. void RecordEvalCall() { if (!is_global_scope()) scope_calls_eval_ = true; } + // Inform the scope that the corresponding code uses "this". + void RecordThisUsage() { scope_uses_this_ = true; } + + // Inform the scope that the corresponding code uses "arguments". + void RecordArgumentsUsage() { scope_uses_arguments_ = true; } + // Set the strict mode flag (unless disabled by a global flag). void SetStrictMode(StrictMode strict_mode) { strict_mode_ = strict_mode; } @@ -262,12 +268,15 @@ class Scope: public ZoneObject { // Specific scope types. bool is_eval_scope() const { return scope_type_ == EVAL_SCOPE; } - bool is_function_scope() const { return scope_type_ == FUNCTION_SCOPE; } + bool is_function_scope() const { + return scope_type_ == FUNCTION_SCOPE || scope_type_ == ARROW_SCOPE; + } bool is_module_scope() const { return scope_type_ == MODULE_SCOPE; } bool is_global_scope() const { return scope_type_ == GLOBAL_SCOPE; } bool is_catch_scope() const { return scope_type_ == CATCH_SCOPE; } bool is_block_scope() const { return scope_type_ == BLOCK_SCOPE; } bool is_with_scope() const { return scope_type_ == WITH_SCOPE; } + bool is_arrow_scope() const { return scope_type_ == ARROW_SCOPE; } bool is_declaration_scope() const { return is_eval_scope() || is_function_scope() || is_module_scope() || is_global_scope(); @@ -292,6 +301,15 @@ class Scope: public ZoneObject { // Does this scope contain a with statement. bool contains_with() const { return scope_contains_with_; } + // Does this scope access "this". + bool uses_this() const { return scope_uses_this_; } + // Does any inner scope access "this". + bool inner_uses_this() const { return inner_scope_uses_this_; } + // 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_; } + // --------------------------------------------------------------------------- // Accessors. @@ -468,6 +486,10 @@ 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 "this". + bool scope_uses_this_; + // This scope uses "arguments". + bool scope_uses_arguments_; // This scope contains an "use asm" annotation. bool asm_module_; // This scope's outer context is an asm module. @@ -481,6 +503,8 @@ class Scope: public ZoneObject { // Computed via PropagateScopeInfo. bool outer_scope_calls_sloppy_eval_; bool inner_scope_calls_eval_; + bool inner_scope_uses_this_; + bool inner_scope_uses_arguments_; bool force_eager_compilation_; bool force_context_allocation_; diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index 3556c64..d33d037 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -917,6 +917,122 @@ static int Utf8LengthHelper(const char* s) { } +TEST(ScopeUsesThisAndArguments) { + static const struct { + const char* prefix; + const char* suffix; + } surroundings[] = { + { "function f() {", "}" }, + { "var f = () => {", "}" }, + }; + + static const struct { + const char* body; + bool uses_this; + bool uses_arguments; + bool inner_uses_this; + bool inner_uses_arguments; + } source_data[] = { + { "", + false, false, false, false }, + { "return this", + true, false, false, false }, + { "return arguments", + false, true, false, false }, + { "return arguments[0]", + false, true, false, false }, + { "return this + arguments[0]", + true, true, false, false }, + { "return x => this + x", + false, false, true, false }, + { "this.foo = 42;", + true, false, false, false }, + { "this.foo();", + true, false, false, false }, + { "if (foo()) { this.f() }", + true, false, false, false }, + { "if (arguments.length) { this.f() }", + true, true, false, false }, + { "while (true) { this.f() }", + true, false, false, false }, + { "if (true) { while (true) this.foo(arguments) }", + true, true, false, false }, + // Multiple nesting levels must work as well. + { "while (true) { while (true) { while (true) return this } }", + true, false, false, false }, + { "if (1) { return () => { while (true) new this() } }", + false, false, true, false }, + // Note that propagation of the inner_uses_this() value does not + // cross boundaries of normal functions onto parent scopes. + { "return function (x) { return this + x }", + false, false, false, false }, + { "var x = function () { this.foo = 42 };", + false, false, false, false }, + { "if (1) { return function () { while (true) new this() } }", + false, false, false, false }, + { "return function (x) { return () => this }", + false, false, false, false }, + // Flags must be correctly set when using block scoping. + { "\"use strict\"; while (true) { let x; this, arguments; }", + false, false, true, true }, + { "\"use strict\"; if (foo()) { let x; this.f() }", + false, false, true, false }, + { "\"use strict\"; if (1) {" + " let x; return function () { return this + arguments }" + "}", + false, false, false, false }, + }; + + i::Isolate* isolate = CcTest::i_isolate(); + i::Factory* factory = isolate->factory(); + + v8::HandleScope handles(CcTest::isolate()); + v8::Handle context = v8::Context::New(CcTest::isolate()); + v8::Context::Scope context_scope(context); + + isolate->stack_guard()->SetStackLimit(i::GetCurrentStackPosition() - + 128 * 1024); + + for (unsigned j = 0; j < arraysize(surroundings); ++j) { + for (unsigned i = 0; i < arraysize(source_data); ++i) { + int kProgramByteSize = i::StrLength(surroundings[j].prefix) + + i::StrLength(surroundings[j].suffix) + + i::StrLength(source_data[i].body); + i::ScopedVector program(kProgramByteSize + 1); + i::SNPrintF(program, "%s%s%s", surroundings[j].prefix, + source_data[i].body, surroundings[j].suffix); + i::Handle source = + factory->NewStringFromUtf8(i::CStrVector(program.start())) + .ToHandleChecked(); + i::Handle script = factory->NewScript(source); + i::CompilationInfoWithZone info(script); + i::Parser::ParseInfo parse_info = {isolate->stack_guard()->real_climit(), + isolate->heap()->HashSeed(), + isolate->unicode_cache()}; + i::Parser parser(&info, &parse_info); + parser.set_allow_arrow_functions(true); + parser.set_allow_harmony_scoping(true); + info.MarkAsGlobal(); + parser.Parse(); + CHECK(i::Rewriter::Rewrite(&info)); + CHECK(i::Scope::Analyze(&info)); + CHECK(info.function() != NULL); + + i::Scope* global_scope = info.function()->scope(); + CHECK(global_scope->is_global_scope()); + CHECK_EQ(1, global_scope->inner_scopes()->length()); + + i::Scope* scope = global_scope->inner_scopes()->at(0); + CHECK_EQ(source_data[i].uses_this, scope->uses_this()); + CHECK_EQ(source_data[i].uses_arguments, scope->uses_arguments()); + CHECK_EQ(source_data[i].inner_uses_this, scope->inner_uses_this()); + CHECK_EQ(source_data[i].inner_uses_arguments, + scope->inner_uses_arguments()); + } + } +} + + TEST(ScopePositions) { v8::internal::FLAG_harmony_scoping = true; @@ -974,11 +1090,10 @@ TEST(ScopePositions) { " infunction;\n" " }", "\n" " more;", i::FUNCTION_SCOPE, i::SLOPPY }, - // TODO(aperez): Change to use i::ARROW_SCOPE when implemented { " start;\n", "(a,b) => a + b", "; more;", - i::FUNCTION_SCOPE, i::SLOPPY }, + i::ARROW_SCOPE, i::SLOPPY }, { " start;\n", "(a,b) => { return a+b; }", "\nmore;", - i::FUNCTION_SCOPE, i::SLOPPY }, + i::ARROW_SCOPE, i::SLOPPY }, { " start;\n" " (function fun", "(a,b) { infunction; }", ")();", i::FUNCTION_SCOPE, i::SLOPPY }, -- 2.7.4