From bbcb33c773d46eebe65e36c1ef4ed5e5b9196fce Mon Sep 17 00:00:00 2001 From: marja Date: Mon, 16 Jan 2017 04:07:57 -0800 Subject: [PATCH] PreParser scope analysis: sloppy block funcs. - Generalize the sloppy block function data structures to allow PreParser adding and hoisting sloppy block funcs. - This completes PreParser scope analysis. BUG=v8:5501, v8:5516 R=verwaest@chromium.org Review-Url: https://codereview.chromium.org/2636543002 Cr-Commit-Position: refs/heads/master@{#42368} --- src/ast/ast.h | 17 ++----- src/ast/scopes.cc | 96 +++++++++++++++++++++++++------------ src/ast/scopes.h | 29 ++++++++--- src/parsing/parser-base.h | 19 +++++++- src/parsing/parser.cc | 26 ++++------ src/parsing/parser.h | 5 +- src/parsing/preparser.cc | 8 ++++ src/parsing/preparser.h | 25 ++++++++-- test/cctest/test-parsing.cc | 29 +++++++++-- 9 files changed, 175 insertions(+), 79 deletions(-) diff --git a/src/ast/ast.h b/src/ast/ast.h index b6aecb1f30..af561e0a3f 100644 --- a/src/ast/ast.h +++ b/src/ast/ast.h @@ -1185,22 +1185,15 @@ class SloppyBlockFunctionStatement final : public Statement { public: Statement* statement() const { return statement_; } void set_statement(Statement* statement) { statement_ = statement; } - Scope* scope() const { return scope_; } - SloppyBlockFunctionStatement* next() { return next_; } - void set_next(SloppyBlockFunctionStatement* next) { next_ = next; } private: friend class AstNodeFactory; - SloppyBlockFunctionStatement(Statement* statement, Scope* scope) + explicit SloppyBlockFunctionStatement(Statement* statement) : Statement(kNoSourcePosition, kSloppyBlockFunctionStatement), - statement_(statement), - scope_(scope), - next_(nullptr) {} + statement_(statement) {} Statement* statement_; - Scope* const scope_; - SloppyBlockFunctionStatement* next_; }; @@ -3290,9 +3283,9 @@ class AstNodeFactory final BASE_EMBEDDED { return new (zone_) EmptyStatement(pos); } - SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement(Scope* scope) { - return new (zone_) SloppyBlockFunctionStatement( - NewEmptyStatement(kNoSourcePosition), scope); + SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement() { + return new (zone_) + SloppyBlockFunctionStatement(NewEmptyStatement(kNoSourcePosition)); } CaseClause* NewCaseClause( diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index 9209c2bb41..8be4d1722a 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -20,6 +20,14 @@ namespace internal { namespace { void* kDummyPreParserVariable = reinterpret_cast(0x1); +void* kDummyPreParserLexicalVariable = reinterpret_cast(0x2); + +bool IsLexical(Variable* variable) { + if (variable == kDummyPreParserLexicalVariable) return true; + if (variable == kDummyPreParserVariable) return false; + return IsLexicalVariableMode(variable->mode()); +} + } // namespace // ---------------------------------------------------------------------------- @@ -56,14 +64,16 @@ Variable* VariableMap::Declare(Zone* zone, Scope* scope, return reinterpret_cast(p->value); } -void VariableMap::DeclareName(Zone* zone, const AstRawString* name) { +void VariableMap::DeclareName(Zone* zone, const AstRawString* name, + VariableMode mode) { Entry* p = ZoneHashMap::LookupOrInsert(const_cast(name), name->hash(), ZoneAllocationPolicy(zone)); if (p->value == nullptr) { // The variable has not been declared yet -> insert it. DCHECK_EQ(name, p->key); - p->value = kDummyPreParserVariable; + p->value = + mode == VAR ? kDummyPreParserVariable : kDummyPreParserLexicalVariable; } } @@ -92,21 +102,27 @@ Variable* VariableMap::Lookup(const AstRawString* name) { return NULL; } +void SloppyBlockFunctionMap::Delegate::set_statement(Statement* statement) { + if (statement_ != nullptr) { + statement_->set_statement(statement); + } +} + SloppyBlockFunctionMap::SloppyBlockFunctionMap(Zone* zone) : ZoneHashMap(8, ZoneAllocationPolicy(zone)) {} -void SloppyBlockFunctionMap::Declare(Zone* zone, const AstRawString* name, - SloppyBlockFunctionStatement* stmt) { +void SloppyBlockFunctionMap::Declare( + Zone* zone, const AstRawString* name, + SloppyBlockFunctionMap::Delegate* delegate) { // AstRawStrings are unambiguous, i.e., the same string is always represented // by the same AstRawString*. Entry* p = ZoneHashMap::LookupOrInsert(const_cast(name), name->hash(), ZoneAllocationPolicy(zone)); - stmt->set_next(static_cast(p->value)); - p->value = stmt; + delegate->set_next(static_cast(p->value)); + p->value = delegate; } - // ---------------------------------------------------------------------------- // Implementation of Scope @@ -448,11 +464,21 @@ int Scope::num_parameters() const { return is_declaration_scope() ? AsDeclarationScope()->num_parameters() : 0; } +void DeclarationScope::DeclareSloppyBlockFunction( + const AstRawString* name, Scope* scope, + SloppyBlockFunctionStatement* statement) { + auto* delegate = + new (zone()) SloppyBlockFunctionMap::Delegate(scope, statement); + sloppy_block_function_map_.Declare(zone(), name, delegate); +} + void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) { DCHECK(is_sloppy(language_mode())); DCHECK(is_function_scope() || is_eval_scope() || is_script_scope() || (is_block_scope() && outer_scope()->is_function_scope())); - DCHECK(HasSimpleParameters() || is_block_scope()); + DCHECK(HasSimpleParameters() || is_block_scope() || is_being_lazily_parsed_); + DCHECK_EQ(factory == nullptr, is_being_lazily_parsed_); + bool has_simple_parameters = HasSimpleParameters(); // For each variable which is used as a function declaration in a sloppy // block, @@ -484,7 +510,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) { bool var_created = false; // Write in assignments to var for each block-scoped function declaration - auto delegates = static_cast(p->value); + auto delegates = static_cast(p->value); DeclarationScope* decl_scope = this; while (decl_scope->is_eval_scope()) { @@ -492,7 +518,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) { } Scope* outer_scope = decl_scope->outer_scope(); - for (SloppyBlockFunctionStatement* delegate = delegates; + for (SloppyBlockFunctionMap::Delegate* delegate = delegates; delegate != nullptr; delegate = delegate->next()) { // Check if there's a conflict with a lexical declaration Scope* query_scope = delegate->scope()->outer_scope(); @@ -506,7 +532,7 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) { // `{ let e; try {} catch (e) { function e(){} } }` do { var = query_scope->LookupLocal(name); - if (var != nullptr && IsLexicalVariableMode(var->mode())) { + if (var != nullptr && IsLexical(var)) { should_hoist = false; break; } @@ -518,25 +544,32 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) { // Declare a var-style binding for the function in the outer scope if (!var_created) { var_created = true; - VariableProxy* proxy = factory->NewVariableProxy(name, NORMAL_VARIABLE); - Declaration* declaration = - factory->NewVariableDeclaration(proxy, this, kNoSourcePosition); - // Based on the preceding check, it doesn't matter what we pass as - // allow_harmony_restrictive_generators and - // sloppy_mode_block_scope_function_redefinition. - bool ok = true; - DeclareVariable(declaration, VAR, - Variable::DefaultInitializationFlag(VAR), false, - nullptr, &ok); - CHECK(ok); // Based on the preceding check, this should not fail + if (factory) { + VariableProxy* proxy = + factory->NewVariableProxy(name, NORMAL_VARIABLE); + auto declaration = + factory->NewVariableDeclaration(proxy, this, kNoSourcePosition); + // Based on the preceding check, it doesn't matter what we pass as + // allow_harmony_restrictive_generators and + // sloppy_mode_block_scope_function_redefinition. + bool ok = true; + DeclareVariable(declaration, VAR, + Variable::DefaultInitializationFlag(VAR), false, + nullptr, &ok); + CHECK(ok); // Based on the preceding check, this should not fail + } else { + DeclareVariableName(name, VAR); + } } - Expression* assignment = factory->NewAssignment( - Token::ASSIGN, NewUnresolved(factory, name), - delegate->scope()->NewUnresolved(factory, name), kNoSourcePosition); - Statement* statement = - factory->NewExpressionStatement(assignment, kNoSourcePosition); - delegate->set_statement(statement); + if (factory) { + Expression* assignment = factory->NewAssignment( + Token::ASSIGN, NewUnresolved(factory, name), + delegate->scope()->NewUnresolved(factory, name), kNoSourcePosition); + Statement* statement = + factory->NewExpressionStatement(assignment, kNoSourcePosition); + delegate->set_statement(statement); + } } } } @@ -1048,7 +1081,7 @@ void Scope::DeclareVariableName(const AstRawString* name, VariableMode mode) { DCHECK(scope_info_.is_null()); // Declare the variable in the declaration scope. - variables_.DeclareName(zone(), name); + variables_.DeclareName(zone(), name, mode); } VariableProxy* Scope::NewUnresolved(AstNodeFactory* factory, @@ -1647,7 +1680,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) { if (var == nullptr) return var; // TODO(marja): Separate LookupRecursive for preparsed scopes better. - if (var == kDummyPreParserVariable) { + if (var == kDummyPreParserVariable || var == kDummyPreParserLexicalVariable) { DCHECK(GetDeclarationScope()->is_being_lazily_parsed()); DCHECK(FLAG_lazy_inner_functions); return var; @@ -1845,7 +1878,8 @@ VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope, if (var == nullptr) { proxy->set_next_unresolved(stack); stack = proxy; - } else if (var != kDummyPreParserVariable) { + } else if (var != kDummyPreParserVariable && + var != kDummyPreParserLexicalVariable) { if (info != nullptr) { // In this case we need to leave scopes in a way that they can be // allocated. If we resolved variables from lazy parsed scopes, we need diff --git a/src/ast/scopes.h b/src/ast/scopes.h index 6f3702132a..d686e7d4b6 100644 --- a/src/ast/scopes.h +++ b/src/ast/scopes.h @@ -21,6 +21,7 @@ class AstRawString; class Declaration; class ParseInfo; class SloppyBlockFunctionStatement; +class Statement; class StringSet; class VariableProxy; @@ -37,7 +38,7 @@ class VariableMap: public ZoneHashMap { // Records that "name" exists (if not recorded yet) but doesn't create a // Variable. Useful for preparsing. - void DeclareName(Zone* zone, const AstRawString* name); + void DeclareName(Zone* zone, const AstRawString* name, VariableMode mode); Variable* Lookup(const AstRawString* name); void Remove(Variable* var); @@ -48,9 +49,24 @@ class VariableMap: public ZoneHashMap { // Sloppy block-scoped function declarations to var-bind class SloppyBlockFunctionMap : public ZoneHashMap { public: + class Delegate : public ZoneObject { + public: + explicit Delegate(Scope* scope, + SloppyBlockFunctionStatement* statement = nullptr) + : scope_(scope), statement_(statement), next_(nullptr) {} + void set_statement(Statement* statement); + void set_next(Delegate* next) { next_ = next; } + Delegate* next() const { return next_; } + Scope* scope() const { return scope_; } + + private: + Scope* scope_; + SloppyBlockFunctionStatement* statement_; + Delegate* next_; + }; + explicit SloppyBlockFunctionMap(Zone* zone); - void Declare(Zone* zone, const AstRawString* name, - SloppyBlockFunctionStatement* statement); + void Declare(Zone* zone, const AstRawString* name, Delegate* delegate); }; enum class AnalyzeMode { kRegular, kDebugger }; @@ -747,10 +763,9 @@ class DeclarationScope : public Scope { // initializers. void AddLocal(Variable* var); - void DeclareSloppyBlockFunction(const AstRawString* name, - SloppyBlockFunctionStatement* statement) { - sloppy_block_function_map_.Declare(zone(), name, statement); - } + void DeclareSloppyBlockFunction( + const AstRawString* name, Scope* scope, + SloppyBlockFunctionStatement* statement = nullptr); // Go through sloppy_block_function_map_ and hoist those (into this scope) // which should be hoisted. diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index d8705a654f..5ce82407ca 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -3780,8 +3780,23 @@ ParserBase::ParseHoistableDeclaration( pos, FunctionLiteral::kDeclaration, language_mode(), CHECK_OK_CUSTOM(NullStatement)); - return impl()->DeclareFunction(variable_name, function, pos, is_generator, - is_async, names, ok); + // In ES6, a function behaves as a lexical binding, except in + // a script scope, or the initial scope of eval or another function. + VariableMode mode = + (!scope()->is_declaration_scope() || scope()->is_module_scope()) ? LET + : VAR; + // Async functions don't undergo sloppy mode block scoped hoisting, and don't + // allow duplicates in a block. Both are represented by the + // sloppy_block_function_map. Don't add them to the map for async functions. + // Generators are also supposed to be prohibited; currently doing this behind + // a flag and UseCounting violations to assess web compatibility. + bool is_sloppy_block_function = + is_sloppy(language_mode()) && !scope()->is_declaration_scope() && + !is_async && !(allow_harmony_restrictive_generators() && is_generator); + + return impl()->DeclareFunction(variable_name, function, mode, pos, + is_generator, is_async, + is_sloppy_block_function, names, ok); } template diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index 3a836a94de..1979f054c1 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -1472,15 +1472,11 @@ void Parser::DeclareAndInitializeVariables( } Statement* Parser::DeclareFunction(const AstRawString* variable_name, - FunctionLiteral* function, int pos, - bool is_generator, bool is_async, + FunctionLiteral* function, VariableMode mode, + int pos, bool is_generator, bool is_async, + bool is_sloppy_block_function, ZoneList* names, bool* ok) { - // In ES6, a function behaves as a lexical binding, except in - // a script scope, or the initial scope of eval or another function. - VariableMode mode = - (!scope()->is_declaration_scope() || scope()->is_module_scope()) ? LET - : VAR; VariableProxy* proxy = factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE); Declaration* declaration = @@ -1488,18 +1484,12 @@ Statement* Parser::DeclareFunction(const AstRawString* variable_name, Declare(declaration, DeclarationDescriptor::NORMAL, mode, kCreatedInitialized, CHECK_OK); if (names) names->Add(variable_name, zone()); - // Async functions don't undergo sloppy mode block scoped hoisting, and don't - // allow duplicates in a block. Both are represented by the - // sloppy_block_function_map. Don't add them to the map for async functions. - // Generators are also supposed to be prohibited; currently doing this behind - // a flag and UseCounting violations to assess web compatibility. - if (is_sloppy(language_mode()) && !scope()->is_declaration_scope() && - !is_async && !(allow_harmony_restrictive_generators() && is_generator)) { - SloppyBlockFunctionStatement* delegate = - factory()->NewSloppyBlockFunctionStatement(scope()); + if (is_sloppy_block_function) { + SloppyBlockFunctionStatement* statement = + factory()->NewSloppyBlockFunctionStatement(); DeclarationScope* target_scope = GetDeclarationScope(); - target_scope->DeclareSloppyBlockFunction(variable_name, delegate); - return delegate; + target_scope->DeclareSloppyBlockFunction(variable_name, scope(), statement); + return statement; } return factory()->NewEmptyStatement(kNoSourcePosition); } diff --git a/src/parsing/parser.h b/src/parsing/parser.h index 45ef959ed4..a898511b23 100644 --- a/src/parsing/parser.h +++ b/src/parsing/parser.h @@ -341,8 +341,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase) { const CatchInfo& catch_info, int pos); Statement* DeclareFunction(const AstRawString* variable_name, - FunctionLiteral* function, int pos, - bool is_generator, bool is_async, + FunctionLiteral* function, VariableMode mode, + int pos, bool is_generator, bool is_async, + bool is_sloppy_block_function, ZoneList* names, bool* ok); V8_INLINE Statement* DeclareClass(const AstRawString* variable_name, Expression* value, diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc index 7f859d2bbe..1dae5e9b66 100644 --- a/src/parsing/preparser.cc +++ b/src/parsing/preparser.cc @@ -144,6 +144,10 @@ PreParser::PreParseResult PreParser::PreParseFunction( LazyParsingResult result = ParseStatementListAndLogFunction( &formals, has_duplicate_parameters, may_abort, ok); + if (is_sloppy(function_scope->language_mode())) { + function_scope->HoistSloppyBlockFunctions(nullptr); + } + use_counts_ = nullptr; track_unresolved_variables_ = false; @@ -232,6 +236,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral( // Parsing the body may change the language mode in our scope. language_mode = function_scope->language_mode(); + if (is_sloppy(language_mode)) { + function_scope->HoistSloppyBlockFunctions(nullptr); + } + // Validate name and parameter names. We can do this only after parsing the // function, since the function can declare itself strict. CheckFunctionName(language_mode, function_name, function_name_validity, diff --git a/src/parsing/preparser.h b/src/parsing/preparser.h index 1f822a46c3..77fe061f42 100644 --- a/src/parsing/preparser.h +++ b/src/parsing/preparser.h @@ -886,6 +886,9 @@ class PreParser : public ParserBase { bool is_module = false) { DCHECK_NULL(scope_state_); DeclarationScope* scope = NewScriptScope(); +#ifdef DEBUG + scope->set_is_being_lazily_parsed(true); +#endif // ModuleDeclarationInstantiation for Source Text Module Records creates a // new Module Environment Record whose outer lexical environment record is @@ -1089,9 +1092,19 @@ class PreParser : public ParserBase { } V8_INLINE PreParserStatement DeclareFunction( - PreParserIdentifier variable_name, PreParserExpression function, int pos, - bool is_generator, bool is_async, ZoneList* names, + PreParserIdentifier variable_name, PreParserExpression function, + VariableMode mode, int pos, bool is_generator, bool is_async, + bool is_sloppy_block_function, ZoneList* names, bool* ok) { + DCHECK_NULL(names); + if (variable_name.string_ != nullptr) { + DCHECK(track_unresolved_variables_); + scope()->DeclareVariableName(variable_name.string_, mode); + if (is_sloppy_block_function) { + GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name.string_, + scope()); + } + } return Statement::Default(); } @@ -1616,8 +1629,8 @@ PreParserStatementList PreParser::ParseEagerFunctionBody( FunctionLiteral::FunctionType function_type, bool* ok) { PreParserStatementList result; - Scope* inner_scope = scope(); - if (!parameters.is_simple) inner_scope = NewScope(BLOCK_SCOPE); + DeclarationScope* inner_scope = scope()->AsDeclarationScope(); + if (!parameters.is_simple) inner_scope = NewVarblockScope(); { BlockState block_state(&scope_state_, inner_scope); @@ -1626,6 +1639,10 @@ PreParserStatementList PreParser::ParseEagerFunctionBody( } Expect(Token::RBRACE, ok); + + if (is_sloppy(inner_scope->language_mode())) { + inner_scope->HoistSloppyBlockFunctions(nullptr); + } return result; } diff --git a/test/cctest/test-parsing.cc b/test/cctest/test-parsing.cc index a3ee6bea04..84869ca0cc 100644 --- a/test/cctest/test-parsing.cc +++ b/test/cctest/test-parsing.cc @@ -8615,6 +8615,21 @@ TEST(NoPessimisticContextAllocation) { {"function inner() { for (let my_var = 0; my_var < 1; ++my_var) { } " "my_var }", true}, + {"function inner() { 'use strict'; if (true) { function my_var() {} } " + "my_var; }", + true}, + {"function inner() { 'use strict'; function inner2() { if (true) { " + "function my_var() {} } my_var; } }", + true}, + {"function inner() { function inner2() { 'use strict'; if (true) { " + "function my_var() {} } my_var; } }", + true}, + {"function inner() { () => { 'use strict'; if (true) { function my_var() " + "{} } my_var; } }", + true}, + {"function inner() { if (true) { let my_var; if (true) { function " + "my_var() {} } } my_var; }", + true}, // No pessimistic context allocation: {"function inner() { var my_var; my_var; }", false}, {"function inner() { var my_var; }", false}, @@ -8806,10 +8821,18 @@ TEST(NoPessimisticContextAllocation) { "my_var } }", false}, {"function inner() { class my_var {}; my_var }", false}, - // In the following cases we still context allocate pessimistically: - {"function inner() { function my_var() {} my_var; }", true}, + {"function inner() { function my_var() {} my_var; }", false}, {"function inner() { if (true) { function my_var() {} } my_var; }", - true}, + false}, + {"function inner() { function inner2() { if (true) { function my_var() " + "{} } my_var; } }", + false}, + {"function inner() { () => { if (true) { function my_var() {} } my_var; " + "} }", + false}, + {"function inner() { if (true) { var my_var; if (true) { function " + "my_var() {} } } my_var; }", + false}, }; for (unsigned i = 0; i < arraysize(inners); ++i) {