From 2579d0bf55f8360f1819a4c352ff65879ed44659 Mon Sep 17 00:00:00 2001 From: Marcin Gasperowicz Date: Thu, 4 Jun 2020 14:48:36 +0200 Subject: [PATCH] LibJS: Hoist function declarations This patch adds function declaration hoisting. The mechanism is similar to var hoisting. Hoisted function declarations are to be put before the hoisted var declarations, hence they have to be treated separately. --- Libraries/LibJS/AST.cpp | 9 ++++-- Libraries/LibJS/AST.h | 7 ++-- Libraries/LibJS/Interpreter.cpp | 6 ++++ Libraries/LibJS/Parser.cpp | 28 ++++++++++------ Libraries/LibJS/Parser.h | 1 + Libraries/LibJS/Tests/function-hoisting.js | 37 ++++++++++++++++++++++ 6 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 Libraries/LibJS/Tests/function-hoisting.js diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index 7cc263b93c..1aebc91944 100644 --- a/Libraries/LibJS/AST.cpp +++ b/Libraries/LibJS/AST.cpp @@ -67,10 +67,8 @@ Value ScopeNode::execute(Interpreter& interpreter) const return interpreter.run(*this); } -Value FunctionDeclaration::execute(Interpreter& interpreter) const +Value FunctionDeclaration::execute(Interpreter&) const { - auto* function = ScriptFunction::create(interpreter.global_object(), name(), body(), parameters(), function_length(), interpreter.current_environment()); - interpreter.set_variable(name(), function); return js_undefined(); } @@ -1765,4 +1763,9 @@ void ScopeNode::add_variables(NonnullRefPtrVector variables m_variables.append(move(variables)); } +void ScopeNode::add_functions(NonnullRefPtrVector functions) +{ + m_functions.append(move(functions)); +} + } diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h index 64f8fd4b96..82b6cf979e 100644 --- a/Libraries/LibJS/AST.h +++ b/Libraries/LibJS/AST.h @@ -40,6 +40,7 @@ namespace JS { class VariableDeclaration; +class FunctionDeclaration; template static inline NonnullRefPtr @@ -124,8 +125,9 @@ public: virtual void dump(int indent) const override; void add_variables(NonnullRefPtrVector); + void add_functions(NonnullRefPtrVector); const NonnullRefPtrVector& variables() const { return m_variables; } - + const NonnullRefPtrVector& functions() const { return m_functions; } bool in_strict_mode() const { return m_strict_mode; } void set_strict_mode() { m_strict_mode = true; } @@ -136,6 +138,7 @@ private: virtual bool is_scope_node() const final { return true; } NonnullRefPtrVector m_children; NonnullRefPtrVector m_variables; + NonnullRefPtrVector m_functions; bool m_strict_mode { false }; }; @@ -175,6 +178,7 @@ public: const FlyString& name() const { return m_name; } const Statement& body() const { return *m_body; } const Vector& parameters() const { return m_parameters; }; + i32 function_length() const { return m_function_length; } protected: FunctionNode(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables) @@ -189,7 +193,6 @@ protected: void dump(int indent, const char* class_name) const; const NonnullRefPtrVector& variables() const { return m_variables; } - i32 function_length() const { return m_function_length; } private: FlyString m_name; diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp index c51e586488..35f39cf035 100644 --- a/Libraries/LibJS/Interpreter.cpp +++ b/Libraries/LibJS/Interpreter.cpp @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -91,6 +92,11 @@ Value Interpreter::run(const Statement& statement, ArgumentVector arguments, Sco void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type) { + for (auto& declaration : scope_node.functions()) { + auto* function = ScriptFunction::create(global_object(), declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment()); + set_variable(declaration.name(), function); + } + if (scope_type == ScopeType::Function) { m_scope_stack.append({ scope_type, scope_node, false }); return; diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index a74e042b6d..1739c7c2b8 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -37,6 +37,7 @@ public: enum Type { Var = 1, Let = 2, + Function = 3, }; ScopePusher(Parser& parser, unsigned mask) @@ -47,6 +48,8 @@ public: m_parser.m_parser_state.m_var_scopes.append(NonnullRefPtrVector()); if (m_mask & Let) m_parser.m_parser_state.m_let_scopes.append(NonnullRefPtrVector()); + if (m_mask & Function) + m_parser.m_parser_state.m_function_scopes.append(NonnullRefPtrVector()); } ~ScopePusher() @@ -55,6 +58,8 @@ public: m_parser.m_parser_state.m_var_scopes.take_last(); if (m_mask & Let) m_parser.m_parser_state.m_let_scopes.take_last(); + if (m_mask & Function) + m_parser.m_parser_state.m_function_scopes.take_last(); } Parser& m_parser; @@ -204,7 +209,7 @@ Associativity Parser::operator_associativity(TokenType type) const NonnullRefPtr Parser::parse_program() { - ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let); + ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Let | ScopePusher::Function); auto program = adopt(*new Program); bool first = true; @@ -228,6 +233,7 @@ NonnullRefPtr Parser::parse_program() if (m_parser_state.m_var_scopes.size() == 1) { program->add_variables(m_parser_state.m_var_scopes.last()); program->add_variables(m_parser_state.m_let_scopes.last()); + program->add_functions(m_parser_state.m_function_scopes.last()); } else { syntax_error("Unclosed scope"); } @@ -238,8 +244,11 @@ NonnullRefPtr Parser::parse_statement() { auto statement = [this]() -> NonnullRefPtr { switch (m_parser_state.m_current_token.type()) { - case TokenType::Function: - return parse_function_node(); + case TokenType::Function: { + auto declaration = parse_function_node(); + m_parser_state.m_function_scopes.last().append(declaration); + return declaration; + } case TokenType::CurlyOpen: return parse_block_statement(); case TokenType::Return: @@ -590,8 +599,7 @@ NonnullRefPtr Parser::parse_object_expression() syntax_error( "Expected '(' for object getter or setter property", m_parser_state.m_current_token.line_number(), - m_parser_state.m_current_token.line_column() - ); + m_parser_state.m_current_token.line_column()); skip_to_next_property(); continue; } @@ -606,8 +614,7 @@ NonnullRefPtr Parser::parse_object_expression() syntax_error( "Object getter property must have no arguments", m_parser_state.m_current_token.line_number(), - m_parser_state.m_current_token.line_column() - ); + m_parser_state.m_current_token.line_column()); skip_to_next_property(); continue; } @@ -615,8 +622,7 @@ NonnullRefPtr Parser::parse_object_expression() syntax_error( "Object setter property must have one argument", m_parser_state.m_current_token.line_number(), - m_parser_state.m_current_token.line_column() - ); + m_parser_state.m_current_token.line_column()); skip_to_next_property(); continue; } @@ -1059,13 +1065,14 @@ NonnullRefPtr Parser::parse_block_statement() m_parser_state.m_strict_mode = initial_strict_mode_state; consume(TokenType::CurlyClose); block->add_variables(m_parser_state.m_let_scopes.last()); + block->add_functions(m_parser_state.m_function_scopes.last()); return block; } template NonnullRefPtr Parser::parse_function_node(bool check_for_function_and_name) { - ScopePusher scope(*this, ScopePusher::Var); + ScopePusher scope(*this, ScopePusher::Var | ScopePusher::Function); if (check_for_function_and_name) consume(TokenType::Function); @@ -1109,6 +1116,7 @@ NonnullRefPtr Parser::parse_function_node(bool check_for_funct auto body = parse_block_statement(); body->add_variables(m_parser_state.m_var_scopes.last()); + body->add_functions(m_parser_state.m_function_scopes.last()); return create_ast_node(name, move(body), move(parameters), function_length, NonnullRefPtrVector()); } diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h index ac45a52b24..4d36d85f98 100644 --- a/Libraries/LibJS/Parser.h +++ b/Libraries/LibJS/Parser.h @@ -147,6 +147,7 @@ private: Vector m_errors; Vector> m_var_scopes; Vector> m_let_scopes; + Vector> m_function_scopes; UseStrictDirectiveState m_use_strict_directive { UseStrictDirectiveState::None }; bool m_strict_mode { false }; diff --git a/Libraries/LibJS/Tests/function-hoisting.js b/Libraries/LibJS/Tests/function-hoisting.js new file mode 100644 index 0000000000..6cd155567b --- /dev/null +++ b/Libraries/LibJS/Tests/function-hoisting.js @@ -0,0 +1,37 @@ +load("test-common.js"); + +try { + var callHoisted = hoisted(); + function hoisted() { + return true; + } + assert(hoisted() === true); + assert(callHoisted === true); + + { + var callScopedHoisted = scopedHoisted(); + function scopedHoisted() { + return "foo"; + } + assert(scopedHoisted() === "foo"); + assert(callScopedHoisted === "foo"); + } + assert(scopedHoisted() === "foo"); + assert(callScopedHoisted === "foo"); + + const test = () => { + var iife = (function () { + return declaredLater(); + })(); + function declaredLater() { + return "yay"; + } + return iife; + }; + assert(typeof declaredLater === "undefined"); + assert(test() === "yay"); + + console.log("PASS"); +} catch (e) { + console.log("FAIL: " + e); +}