LibJS: Allow using local variable for catch parameters

Local variables are faster to access and if all catch parameters are
locals we can skip lexical environment allocation.
This commit is contained in:
Aliaksandr Kalenik 2025-04-22 00:05:36 +02:00 committed by Alexander Kalenik
parent 0f14c70252
commit 7932091e02
Notes: github-actions[bot] 2025-04-22 19:58:29 +00:00
3 changed files with 49 additions and 28 deletions

View file

@ -2661,17 +2661,35 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> TryStatement::generate_
TRY(m_handler->parameter().visit(
[&](NonnullRefPtr<Identifier const> const& parameter) -> Bytecode::CodeGenerationErrorOr<void> {
generator.begin_variable_scope();
did_create_variable_scope_for_catch_clause = true;
auto parameter_identifier = generator.intern_identifier(parameter->string());
generator.emit<Bytecode::Op::CreateVariable>(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false);
generator.emit<Bytecode::Op::InitializeLexicalBinding>(parameter_identifier, caught_value);
if (parameter->is_local()) {
auto local = generator.local(parameter->local_variable_index());
generator.emit_mov(local, caught_value);
} else {
generator.begin_variable_scope();
did_create_variable_scope_for_catch_clause = true;
auto parameter_identifier = generator.intern_identifier(parameter->string());
generator.emit<Bytecode::Op::CreateVariable>(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false);
generator.emit<Bytecode::Op::InitializeLexicalBinding>(parameter_identifier, caught_value);
}
return {};
},
[&](NonnullRefPtr<BindingPattern const> const& binding_pattern) -> Bytecode::CodeGenerationErrorOr<void> {
generator.begin_variable_scope();
did_create_variable_scope_for_catch_clause = true;
TRY(binding_pattern->generate_bytecode(generator, Bytecode::Op::BindingInitializationMode::Initialize, caught_value, true));
MUST(binding_pattern->for_each_bound_identifier([&](auto const& identifier) {
if (!identifier.is_local())
did_create_variable_scope_for_catch_clause = true;
}));
if (did_create_variable_scope_for_catch_clause)
generator.begin_variable_scope();
MUST(binding_pattern->for_each_bound_identifier([&](auto const& identifier) {
if (identifier.is_local())
return;
auto parameter_identifier = generator.intern_identifier(identifier.string());
generator.emit<Bytecode::Op::CreateVariable>(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false);
}));
TRY(binding_pattern->generate_bytecode(generator, Bytecode::Op::BindingInitializationMode::Initialize, caught_value, false));
return {};
},
[](Empty) -> Bytecode::CodeGenerationErrorOr<void> {

View file

@ -105,21 +105,25 @@ public:
return scope_pusher;
}
static ScopePusher catch_scope(Parser& parser, RefPtr<BindingPattern const> const& pattern, RefPtr<Identifier const> const& parameter)
static ScopePusher catch_scope(Parser& parser)
{
return ScopePusher(parser, nullptr, ScopeLevel::NotTopLevel, ScopeType::Catch);
}
void add_catch_parameter(RefPtr<BindingPattern const> const& pattern, RefPtr<Identifier const> const& parameter)
{
ScopePusher scope_pusher(parser, nullptr, ScopeLevel::NotTopLevel, ScopeType::Catch);
if (pattern) {
// NOTE: Nothing in the callback throws an exception.
MUST(pattern->for_each_bound_identifier([&](auto const& identifier) {
scope_pusher.m_forbidden_var_names.set(identifier.string());
scope_pusher.m_bound_names.set(identifier.string());
m_forbidden_var_names.set(identifier.string());
m_bound_names.set(identifier.string());
m_catch_parameter_names.set(identifier.string());
}));
} else if (parameter) {
scope_pusher.m_var_names.set(parameter->string());
scope_pusher.m_bound_names.set(parameter->string());
m_var_names.set(parameter->string());
m_bound_names.set(parameter->string());
m_catch_parameter_names.set(parameter->string());
}
return scope_pusher;
}
static ScopePusher static_init_block_scope(Parser& parser, ScopeNode& node)
@ -289,12 +293,6 @@ public:
auto const& identifier_group_name = it.key;
auto& identifier_group = it.value;
if (m_parser.m_state.in_catch_parameter_context) {
// NOTE: The parser currently cannot determine if an identifier captured by a function belongs to the environment created by a catch parameter.
// As a result, any identifiers used inside the catch parameter are not considered as candidates for optimization in local or global variable access.
continue;
}
bool scope_has_declaration = false;
if (is_top_level() && m_var_names.contains(identifier_group_name))
scope_has_declaration = true;
@ -304,13 +302,16 @@ public:
if (m_type == ScopeType::Function && !m_is_arrow_function && identifier_group_name == "arguments"sv)
scope_has_declaration = true;
if (m_type == ScopeType::Catch && m_catch_parameter_names.contains(identifier_group_name))
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)) {
if (m_type == ScopeType::ClassDeclaration && m_bound_names.contains(identifier_group_name)) {
// NOTE: Currently, the parser cannot recognize that assigning a named function expression creates a scope with a binding for the function name.
// As a result, function names are not considered as candidates for optimization in global variable access.
continue;
@ -321,8 +322,8 @@ public:
identifier_group.might_be_variable_in_lexical_scope_in_named_function_assignment = true;
}
if (m_type == ScopeType::ClassDeclaration || m_type == ScopeType::Catch) {
// NOTE: Class declaration and catch scopes do not have own ScopeNode hence can't contain declaration of any variable
if (m_type == ScopeType::ClassDeclaration) {
// NOTE: Class declaration doesn't not have own ScopeNode hence can't contain declaration of any variable
scope_has_declaration = false;
}
@ -494,6 +495,7 @@ private:
HashTable<FlyString> m_lexical_names;
HashTable<FlyString> m_var_names;
HashTable<FlyString> m_function_names;
HashTable<FlyString> m_catch_parameter_names;
HashTable<FlyString> m_forbidden_lexical_names;
HashTable<FlyString> m_forbidden_var_names;
@ -3777,11 +3779,12 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
auto rule_start = push_start();
consume(TokenType::Catch);
ScopePusher catch_scope = ScopePusher::catch_scope(*this);
RefPtr<Identifier const> parameter;
RefPtr<BindingPattern const> pattern_parameter;
auto should_expect_parameter = false;
if (match(TokenType::ParenOpen)) {
TemporaryChange catch_parameter_context_change { m_state.in_catch_parameter_context, true };
should_expect_parameter = true;
consume();
if (match_identifier_name()
@ -3814,7 +3817,8 @@ NonnullRefPtr<CatchClause const> Parser::parse_catch_clause()
bound_names.set(parameter->string());
}
ScopePusher catch_scope = ScopePusher::catch_scope(*this, pattern_parameter, parameter);
catch_scope.add_catch_parameter(pattern_parameter, parameter);
auto body = parse_block_statement();
// NOTE: Nothing in the callback throws an exception.

View file

@ -320,7 +320,6 @@ private:
bool initiated_by_eval { false };
bool in_eval_function_context { false }; // This controls if we allow new.target or not. Note that eval("return") is not allowed, so we have to have a separate state variable for eval.
bool in_formal_parameter_context { false };
bool in_catch_parameter_context { false };
bool in_generator_function_context { false };
bool await_expression_is_valid { false };
bool in_arrow_function_context { false };