diff --git a/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 1b2fbd7a921..e1939e2a748 100644 --- a/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -2661,17 +2661,35 @@ Bytecode::CodeGenerationErrorOr> TryStatement::generate_ TRY(m_handler->parameter().visit( [&](NonnullRefPtr const& parameter) -> Bytecode::CodeGenerationErrorOr { - generator.begin_variable_scope(); - did_create_variable_scope_for_catch_clause = true; - auto parameter_identifier = generator.intern_identifier(parameter->string()); - generator.emit(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false); - generator.emit(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(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false); + generator.emit(parameter_identifier, caught_value); + } return {}; }, [&](NonnullRefPtr const& binding_pattern) -> Bytecode::CodeGenerationErrorOr { - 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(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false); + })); + + TRY(binding_pattern->generate_bytecode(generator, Bytecode::Op::BindingInitializationMode::Initialize, caught_value, false)); return {}; }, [](Empty) -> Bytecode::CodeGenerationErrorOr { diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index 67a712bdba6..5654243c6f5 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -105,21 +105,25 @@ public: return scope_pusher; } - static ScopePusher catch_scope(Parser& parser, RefPtr const& pattern, RefPtr const& parameter) + static ScopePusher catch_scope(Parser& parser) + { + return ScopePusher(parser, nullptr, ScopeLevel::NotTopLevel, ScopeType::Catch); + } + + void add_catch_parameter(RefPtr const& pattern, RefPtr 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 m_lexical_names; HashTable m_var_names; HashTable m_function_names; + HashTable m_catch_parameter_names; HashTable m_forbidden_lexical_names; HashTable m_forbidden_var_names; @@ -3777,11 +3779,12 @@ NonnullRefPtr Parser::parse_catch_clause() auto rule_start = push_start(); consume(TokenType::Catch); + ScopePusher catch_scope = ScopePusher::catch_scope(*this); + RefPtr parameter; RefPtr 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 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. diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h index e37c0ac9845..fa8a8daf5c0 100644 --- a/Libraries/LibJS/Parser.h +++ b/Libraries/LibJS/Parser.h @@ -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 };