From 8b6450842e0c0db4a6115098a6218354ce02b637 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sat, 8 Jul 2023 19:31:41 +0200 Subject: [PATCH] LibJS: Use local variables for function declarations when possible Previously, the usage of local variables was limited for all function declarations. This change relaxes the restriction and only prohibits locals for hoistable annexB declarations. --- Userland/Libraries/LibJS/AST.cpp | 8 +++- Userland/Libraries/LibJS/AST.h | 1 + Userland/Libraries/LibJS/Parser.cpp | 47 ++++++++++--------- .../Runtime/ECMAScriptFunctionObject.cpp | 6 ++- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index c964378fac..defb9243b4 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -4769,8 +4769,12 @@ void ScopeNode::block_declaration_instantiation(VM& vm, Environment* environment if (is(declaration)) { auto& function_declaration = static_cast(declaration); auto function = ECMAScriptFunctionObject::create(realm, function_declaration.name(), function_declaration.source_text(), function_declaration.body(), function_declaration.parameters(), function_declaration.function_length(), function_declaration.local_variables_names(), environment, private_environment, function_declaration.kind(), function_declaration.is_strict_mode(), function_declaration.might_need_arguments_object(), function_declaration.contains_direct_call_to_eval()); - VERIFY(is(*environment)); - static_cast(*environment).initialize_or_set_mutable_binding({}, vm, function_declaration.name(), function); + if (vm.bytecode_interpreter_if_exists() && function_declaration.name_identifier()->is_local()) { + vm.running_execution_context().local_variables[function_declaration.name_identifier()->local_variable_index()] = function; + } else { + VERIFY(is(*environment)); + static_cast(*environment).initialize_or_set_mutable_binding({}, vm, function_declaration.name(), function); + } } })); } diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 8284739055..5fb4ae9458 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -707,6 +707,7 @@ struct FunctionParameter { class FunctionNode { public: StringView name() const { return m_name ? m_name->string().view() : ""sv; } + RefPtr name_identifier() const { return m_name; } DeprecatedString const& source_text() const { return m_source_text; } Statement const& body() const { return *m_body; } Vector const& parameters() const { return m_parameters; } diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index 1eff6321e2..7bd6fa01ba 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -283,18 +283,6 @@ public: return; } - for (size_t i = 0; i < m_functions_to_hoist.size(); i++) { - auto const& function_declaration = m_functions_to_hoist[i]; - if (m_lexical_names.contains(function_declaration->name()) || m_forbidden_var_names.contains(function_declaration->name())) - continue; - if (is_top_level()) { - m_node->add_hoisted_function(move(m_functions_to_hoist[i])); - } else { - if (!m_parent_scope->m_lexical_names.contains(function_declaration->name()) && !m_parent_scope->m_function_names.contains(function_declaration->name())) - m_parent_scope->m_functions_to_hoist.append(move(m_functions_to_hoist[i])); - } - } - for (auto& it : m_identifier_groups) { auto const& identifier_group_name = it.key; auto& identifier_group = it.value; @@ -316,20 +304,21 @@ public: scope_has_declaration = true; })); - bool function_declaration = false; MUST(m_node->for_each_var_function_declaration_in_reverse_order([&](auto const& declaration) { if (declaration.name() == identifier_group_name) - function_declaration = true; + scope_has_declaration = true; })); MUST(m_node->for_each_lexical_function_declaration_in_reverse_order([&](auto const& declaration) { if (declaration.name() == identifier_group_name) - function_declaration = true; - })); - MUST(m_node->for_each_function_hoistable_with_annexB_extension([&](auto const& declaration) { - if (declaration.name() == identifier_group_name) - function_declaration = true; + scope_has_declaration = true; })); + bool hoistable_function_declaration = false; + for (auto const& function_declaration : m_functions_to_hoist) { + if (function_declaration->name() == identifier_group_name) + hoistable_function_declaration = true; + } + if ((m_type == ScopeType::ClassDeclaration || m_type == ScopeType::Catch) && m_bound_names.contains(identifier_group_name)) { // NOTE: Currently class names and catch section parameters are not considered to become local variables // but this might be fixed in the future @@ -346,7 +335,7 @@ public: } if (scope_has_declaration) { - if (function_declaration) + if (hoistable_function_declaration) continue; if (!identifier_group.captured_by_nested_function) { @@ -377,6 +366,18 @@ public: } } + for (size_t i = 0; i < m_functions_to_hoist.size(); i++) { + auto const& function_declaration = m_functions_to_hoist[i]; + if (m_lexical_names.contains(function_declaration->name()) || m_forbidden_var_names.contains(function_declaration->name())) + continue; + if (is_top_level()) { + m_node->add_hoisted_function(move(m_functions_to_hoist[i])); + } else { + if (!m_parent_scope->m_lexical_names.contains(function_declaration->name()) && !m_parent_scope->m_function_names.contains(function_declaration->name())) + m_parent_scope->m_functions_to_hoist.append(move(m_functions_to_hoist[i])); + } + } + VERIFY(m_parser.m_state.current_scope_pusher == this); m_parser.m_state.current_scope_pusher = m_parent_scope; } @@ -2799,15 +2800,15 @@ NonnullRefPtr Parser::parse_function_node(u16 parse_options, O } if (parse_options & FunctionNodeParseOptions::HasDefaultExportName) { - name = create_ast_node( + name = create_identifier_and_register_in_current_scope( { m_source_code, rule_start.position(), position() }, ExportStatement::local_name_for_default); } else if (FunctionNodeType::must_have_name() || match_identifier()) { - name = create_ast_node( + name = create_identifier_and_register_in_current_scope( { m_source_code, rule_start.position(), position() }, consume_identifier().DeprecatedFlyString_value()); } else if (is_function_expression && (match(TokenType::Yield) || match(TokenType::Await))) { - name = create_ast_node( + name = create_identifier_and_register_in_current_scope( { m_source_code, rule_start.position(), position() }, consume().DeprecatedFlyString_value()); } diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index 54607e3ddb..9f4ac1b53a 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -616,7 +616,11 @@ ThrowCompletionOr ECMAScriptFunctionObject::function_declaration_instantia auto private_environment = callee_context.private_environment; for (auto& declaration : functions_to_initialize) { auto function = ECMAScriptFunctionObject::create(realm, declaration.name(), declaration.source_text(), declaration.body(), declaration.parameters(), declaration.function_length(), declaration.local_variables_names(), lex_environment, private_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.contains_direct_call_to_eval()); - MUST(var_environment->set_mutable_binding(vm, declaration.name(), function, false)); + if ((vm.bytecode_interpreter_if_exists() || kind() == FunctionKind::Generator) && declaration.name_identifier()->is_local()) { + callee_context.local_variables[declaration.name_identifier()->local_variable_index()] = function; + } else { + MUST(var_environment->set_mutable_binding(vm, declaration.name(), function, false)); + } } if (is(*lex_environment))