diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index af35cf7892c..880da05f0f3 100644 --- a/Libraries/LibJS/AST.cpp +++ b/Libraries/LibJS/AST.cpp @@ -1027,7 +1027,7 @@ void Identifier::dump(int indent) const { print_indent(indent); if (is_local()) { - outln("Identifier \"{}\" is_local=(true) index=({})", m_string, m_local_variable_index); + outln("Identifier \"{}\" is_local=(true) index=({})", m_string, m_local_index->index); } else if (is_global()) { outln("Identifier \"{}\" is_global=(true)", m_string); } else { @@ -1668,7 +1668,12 @@ void ScopeNode::block_declaration_instantiation(VM& vm, Environment* environment auto& running_execution_context = vm.running_execution_context(); auto number_of_registers = running_execution_context.executable->number_of_registers; auto number_of_constants = running_execution_context.executable->constants.size(); - running_execution_context.local(function_declaration.name_identifier()->local_variable_index() + number_of_registers + number_of_constants) = function; + auto local_index = function_declaration.name_identifier()->local_index(); + if (local_index.is_variable()) { + running_execution_context.local(local_index.index + number_of_registers + number_of_constants) = function; + } else { + VERIFY_NOT_REACHED(); + } } else { VERIFY(is(*environment)); static_cast(*environment).initialize_or_set_mutable_binding({}, vm, function_declaration.name(), function); diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h index 5ff65ce8f28..cf64ebc26d8 100644 --- a/Libraries/LibJS/AST.h +++ b/Libraries/LibJS/AST.h @@ -675,13 +675,29 @@ public: FlyString const& string() const { return m_string; } - bool is_local() const { return m_local_variable_index.has_value(); } - size_t local_variable_index() const + struct Local { + enum Type { + Argument, + Variable, + }; + Type type; + size_t index; + + bool is_argument() const { return type == Argument; } + bool is_variable() const { return type == Variable; } + + static Local variable(size_t index) { return { Variable, index }; } + static Local argument(size_t index) { return { Argument, index }; } + }; + + bool is_local() const { return m_local_index.has_value(); } + Local local_index() const { - VERIFY(m_local_variable_index.has_value()); - return m_local_variable_index.value(); + VERIFY(m_local_index.has_value()); + return m_local_index.value(); } - void set_local_variable_index(size_t index) { m_local_variable_index = index; } + void set_local_variable_index(size_t index) { m_local_index = Local::variable(index); } + void set_argument_index(size_t index) { m_local_index = Local::argument(index); } bool is_global() const { return m_is_global; } void set_is_global() { m_is_global = true; } @@ -694,7 +710,7 @@ private: FlyString m_string; - Optional m_local_variable_index; + Optional m_local_index; bool m_is_global { false }; }; @@ -719,6 +735,20 @@ public: size_t size() const { return m_parameters.size(); } Vector const& parameters() const { return m_parameters; } + Optional get_index_of_parameter_name(FlyString const& name) const + { + // Iterate backwards to return the last parameter with the same name + for (int i = m_parameters.size() - 1; i >= 0; i--) { + auto& parameter = m_parameters[i]; + if (parameter.binding.has>()) { + auto& identifier = parameter.binding.get>(); + if (identifier->string() == name) + return i; + } + } + return {}; + } + private: FunctionParameters(Vector parameters) : m_parameters(move(parameters)) diff --git a/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 7630a2ef4d3..292b7f63697 100644 --- a/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -461,8 +461,14 @@ Bytecode::CodeGenerationErrorOr> Identifier::generate_by Bytecode::Generator::SourceLocationScope scope(generator, *this); if (is_local()) { - auto local = generator.local(local_variable_index()); - if (!generator.is_local_initialized(local_variable_index())) { + auto local_index = this->local_index(); + auto local = generator.local(local_index); + if (!generator.is_local_initialized(local_index)) { + if (local_index.is_argument()) { + // Arguments are initialized to undefined by default, so here we need to replace it with the empty value to + // trigger the TDZ check. + generator.emit(local, generator.add_constant(js_special_empty_value())); + } generator.emit(local); } return local; @@ -1605,7 +1611,7 @@ Bytecode::CodeGenerationErrorOr> VariableDeclaration::ge if (declaration_kind() != DeclarationKind::Var) { if (auto const* identifier = declarator->target().get_pointer>()) { if ((*identifier)->is_local()) { - init_dst = generator.local((*identifier)->local_variable_index()); + init_dst = generator.local((*identifier)->local_index()); } } } @@ -1625,7 +1631,7 @@ Bytecode::CodeGenerationErrorOr> VariableDeclaration::ge if (auto const* identifier = declarator->target().get_pointer>()) { if ((*identifier)->is_local()) { - generator.set_local_initialized((*identifier)->local_variable_index()); + generator.set_local_initialized((*identifier)->local_index()); } } } @@ -1736,7 +1742,7 @@ Bytecode::CodeGenerationErrorOr> CallExpression::generat call_type = Bytecode::Op::CallType::DirectEval; } if (identifier.is_local()) { - auto local = generator.local(identifier.local_variable_index()); + auto local = generator.local(identifier.local_index()); if (!generator.is_local_initialized(local.operand().index())) { generator.emit(local); } @@ -2648,7 +2654,7 @@ Bytecode::CodeGenerationErrorOr> TryStatement::generate_ TRY(m_handler->parameter().visit( [&](NonnullRefPtr const& parameter) -> Bytecode::CodeGenerationErrorOr { if (parameter->is_local()) { - auto local = generator.local(parameter->local_variable_index()); + auto local = generator.local(parameter->local_index()); generator.emit_mov(local, caught_value); } else { generator.begin_variable_scope(); diff --git a/Libraries/LibJS/Bytecode/Generator.cpp b/Libraries/LibJS/Bytecode/Generator.cpp index de43c27ed7d..207a5f3094b 100644 --- a/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Libraries/LibJS/Bytecode/Generator.cpp @@ -58,7 +58,7 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E Optional dst; auto local_var_index = function.shared_data().m_local_variables_names.find_first_index("arguments"_fly_string); if (local_var_index.has_value()) - dst = local(local_var_index.value()); + dst = local(Identifier::Local::variable(local_var_index.value())); if (function.is_strict_mode() || !function.has_simple_parameter_list()) { emit(dst, Op::CreateArguments::Kind::Unmapped, function.is_strict_mode()); @@ -77,11 +77,8 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E auto& if_undefined_block = make_block(); auto& if_not_undefined_block = make_block(); - auto argument_reg = allocate_register(); - emit(argument_reg.operand(), Operand { Operand::Type::Argument, param_index }); - emit( - argument_reg.operand(), + Operand { Operand::Type::Argument, param_index }, Label { if_undefined_block }, Label { if_not_undefined_block }); @@ -95,9 +92,7 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E if (auto const* identifier = parameter.binding.get_pointer>(); identifier) { if ((*identifier)->is_local()) { - auto local_variable_index = (*identifier)->local_variable_index(); - emit(local(local_variable_index), Operand { Operand::Type::Argument, param_index }); - set_local_initialized((*identifier)->local_variable_index()); + set_local_initialized((*identifier)->local_index()); } else { auto id = intern_identifier((*identifier)->string()); if (function.shared_data().m_has_duplicates) { @@ -122,7 +117,7 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E for (auto const& variable_to_initialize : function.shared_data().m_var_names_to_initialize_binding) { auto const& id = variable_to_initialize.identifier; if (id.is_local()) { - emit(local(id.local_variable_index()), add_constant(js_undefined())); + emit(local(id.local_index()), add_constant(js_undefined())); } else { auto intern_id = intern_identifier(id.string()); emit(intern_id, Op::EnvironmentMode::Var, false); @@ -153,14 +148,14 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E emit(initial_value, add_constant(js_undefined())); } else { if (id.is_local()) { - emit(initial_value, local(id.local_variable_index())); + emit(initial_value, local(id.local_index())); } else { emit(initial_value, intern_identifier(id.string())); } } if (id.is_local()) { - emit(local(id.local_variable_index()), initial_value); + emit(local(id.local_index()), initial_value); } else { auto intern_id = intern_identifier(id.string()); emit(intern_id, Op::EnvironmentMode::Var, false); @@ -204,7 +199,7 @@ CodeGenerationErrorOr Generator::emit_function_declaration_instantiation(E for (auto const& declaration : function.shared_data().m_functions_to_initialize) { auto const& identifier = *declaration.name_identifier(); if (identifier.is_local()) { - auto local_index = identifier.local_variable_index(); + auto local_index = identifier.local_index(); emit(local(local_index), declaration, OptionalNone {}); set_local_initialized(local_index); } else { @@ -516,9 +511,11 @@ void Generator::free_register(Register reg) m_free_registers.append(reg); } -ScopedOperand Generator::local(u32 local_index) +ScopedOperand Generator::local(Identifier::Local const& local) { - return ScopedOperand { *this, Operand { Operand::Type::Local, static_cast(local_index) } }; + if (local.is_variable()) + return ScopedOperand { *this, Operand { Operand::Type::Local, static_cast(local.index) } }; + return ScopedOperand { *this, Operand { Operand::Type::Argument, static_cast(local.index) } }; } Generator::SourceLocationScope::SourceLocationScope(Generator& generator, ASTNode const& node) @@ -880,11 +877,12 @@ CodeGenerationErrorOr> Generator::emit_delete_reference( void Generator::emit_set_variable(JS::Identifier const& identifier, ScopedOperand value, Bytecode::Op::BindingInitializationMode initialization_mode, Bytecode::Op::EnvironmentMode environment_mode) { if (identifier.is_local()) { - if (value.operand().is_local() && value.operand().index() == identifier.local_variable_index()) { + auto local_index = identifier.local_index(); + if (value.operand().is_local() && local_index.is_variable() && value.operand().index() == local_index.index) { // Moving a local to itself is a no-op. return; } - emit(local(identifier.local_variable_index()), value); + emit(local(local_index), value); } else { auto identifier_index = intern_identifier(identifier.string()); if (environment_mode == Bytecode::Op::EnvironmentMode::Lexical) { @@ -1186,9 +1184,24 @@ bool Generator::is_local_initialized(u32 local_index) const return m_initialized_locals.find(local_index) != m_initialized_locals.end(); } -void Generator::set_local_initialized(u32 local_index) +bool Generator::is_local_initialized(Identifier::Local const& local) const { - m_initialized_locals.set(local_index); + if (local.is_variable()) + return m_initialized_locals.find(local.index) != m_initialized_locals.end(); + if (local.is_argument()) + return m_initialized_arguments.find(local.index) != m_initialized_arguments.end(); + return true; +} + +void Generator::set_local_initialized(Identifier::Local const& local) +{ + if (local.is_variable()) { + m_initialized_locals.set(local.index); + } else if (local.is_argument()) { + m_initialized_arguments.set(local.index); + } else { + VERIFY_NOT_REACHED(); + } } ScopedOperand Generator::get_this(Optional preferred_dst) diff --git a/Libraries/LibJS/Bytecode/Generator.h b/Libraries/LibJS/Bytecode/Generator.h index f85279ad9c5..3adfa179d80 100644 --- a/Libraries/LibJS/Bytecode/Generator.h +++ b/Libraries/LibJS/Bytecode/Generator.h @@ -44,14 +44,15 @@ public: CodeGenerationErrorOr emit_function_declaration_instantiation(ECMAScriptFunctionObject const& function); [[nodiscard]] ScopedOperand allocate_register(); - [[nodiscard]] ScopedOperand local(u32 local_index); + [[nodiscard]] ScopedOperand local(Identifier::Local const&); [[nodiscard]] ScopedOperand accumulator(); [[nodiscard]] ScopedOperand this_value(); void free_register(Register); - void set_local_initialized(u32 local_index); + void set_local_initialized(Identifier::Local const&); [[nodiscard]] bool is_local_initialized(u32 local_index) const; + [[nodiscard]] bool is_local_initialized(Identifier::Local const&) const; class SourceLocationScope { public: @@ -411,6 +412,7 @@ private: Vector m_home_objects; HashTable m_initialized_locals; + HashTable m_initialized_arguments; bool m_finished { false }; bool m_must_propagate_completion { true }; diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index fad0d5e6c57..c7787d2d6e7 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -326,9 +326,10 @@ public: scope_has_declaration = false; } + bool is_function_parameter = false; if (m_type == ScopeType::Function) { if (!m_contains_access_to_arguments_object_in_non_strict_mode && m_function_parameters_candidates_for_local_variables.contains(identifier_group_name)) { - scope_has_declaration = true; + is_function_parameter = true; } else if (m_forbidden_lexical_names.contains(identifier_group_name)) { // NOTE: If an identifier is used as a function parameter that cannot be optimized locally or globally, it is simply ignored. continue; @@ -346,7 +347,7 @@ public: for (auto& identifier : identifier_group.identifiers) identifier->set_is_global(); } - } else if (scope_has_declaration) { + } else if (scope_has_declaration || is_function_parameter) { if (hoistable_function_declaration) continue; @@ -368,9 +369,15 @@ public: local_scope = m_top_level_scope; } - auto local_variable_index = local_scope->m_node->add_local_variable(identifier_group_name); - for (auto& identifier : identifier_group.identifiers) - identifier->set_local_variable_index(local_variable_index); + if (is_function_parameter) { + auto argument_index = local_scope->m_function_parameters->get_index_of_parameter_name(identifier_group_name); + for (auto& identifier : identifier_group.identifiers) + identifier->set_argument_index(argument_index.value()); + } else { + auto local_variable_index = local_scope->m_node->add_local_variable(identifier_group_name); + for (auto& identifier : identifier_group.identifiers) + identifier->set_local_variable_index(local_variable_index); + } } } else { if (m_function_parameters || m_type == ScopeType::ClassField || m_type == ScopeType::ClassStaticInit) {