From 8a3c9d98514cd73e30059983089f3a0d33b86819 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 22 Jun 2021 22:20:17 +0200 Subject: LibJS: Remove direct argument loading since it was buggy The parser doesn't always track lexical scopes correctly, so let's not rely on that for direct argument loading. This reverts the LoadArguments bytecode instruction as well. We can bring these things back when the parser can reliably tell us that a given Identifier is indeed a function argument. --- Userland/Libraries/LibJS/AST.cpp | 10 +--------- Userland/Libraries/LibJS/AST.h | 5 +---- Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp | 5 +---- Userland/Libraries/LibJS/Bytecode/Instruction.h | 1 - Userland/Libraries/LibJS/Bytecode/Op.cpp | 10 ---------- Userland/Libraries/LibJS/Bytecode/Op.h | 16 ---------------- Userland/Libraries/LibJS/Parser.cpp | 18 +----------------- Userland/Libraries/LibJS/Runtime/Reference.cpp | 8 -------- Userland/Libraries/LibJS/Runtime/Reference.h | 10 ---------- 9 files changed, 4 insertions(+), 79 deletions(-) (limited to 'Userland/Libraries') diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 57e5c92ee4..b0014cca06 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -665,8 +665,6 @@ Reference Expression::to_reference(Interpreter&, GlobalObject&) const Reference Identifier::to_reference(Interpreter& interpreter, GlobalObject&) const { - if (m_argument_index.has_value()) - return Reference(Reference::CallFrameArgument, m_argument_index.value(), string()); return interpreter.vm().get_reference(string()); } @@ -1315,9 +1313,6 @@ Value Identifier::execute(Interpreter& interpreter, GlobalObject& global_object) { InterpreterNodeScope node_scope { interpreter, *this }; - if (m_argument_index.has_value()) - return interpreter.vm().argument(m_argument_index.value()); - auto value = interpreter.vm().get_variable(string(), global_object); if (value.is_empty()) { if (!interpreter.exception()) @@ -1330,10 +1325,7 @@ Value Identifier::execute(Interpreter& interpreter, GlobalObject& global_object) void Identifier::dump(int indent) const { print_indent(indent); - if (m_argument_index.has_value()) - outln("Identifier \"{}\" (argument #{})", m_string, m_argument_index.value()); - else - outln("Identifier \"{}\"", m_string); + outln("Identifier \"{}\"", m_string); } void SpreadExpression::dump(int indent) const diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 1a1fca4bc9..457a20e8c3 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -776,15 +776,13 @@ private: class Identifier final : public Expression { public: - explicit Identifier(SourceRange source_range, FlyString string, Optional argument_index = {}) + explicit Identifier(SourceRange source_range, FlyString string) : Expression(source_range) , m_string(move(string)) - , m_argument_index(move(argument_index)) { } FlyString const& string() const { return m_string; } - Optional const& argument_index() const { return m_argument_index; } virtual Value execute(Interpreter&, GlobalObject&) const override; virtual void dump(int indent) const override; @@ -795,7 +793,6 @@ private: virtual bool is_identifier() const override { return true; } FlyString m_string; - Optional m_argument_index; }; class ClassMethod final : public ASTNode { diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 77f589f66b..a6029f20cc 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -267,10 +267,7 @@ void RegExpLiteral::generate_bytecode(Bytecode::Generator& generator) const void Identifier::generate_bytecode(Bytecode::Generator& generator) const { - if (m_argument_index.has_value()) - generator.emit(m_argument_index.value()); - else - generator.emit(generator.intern_string(m_string)); + generator.emit(generator.intern_string(m_string)); } void AssignmentExpression::generate_bytecode(Bytecode::Generator& generator) const diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.h b/Userland/Libraries/LibJS/Bytecode/Instruction.h index 58139ffd4b..8c0e9289dc 100644 --- a/Userland/Libraries/LibJS/Bytecode/Instruction.h +++ b/Userland/Libraries/LibJS/Bytecode/Instruction.h @@ -65,7 +65,6 @@ O(Decrement) \ O(Throw) \ O(PushDeclarativeEnvironmentRecord) \ - O(LoadArgument) \ O(EnterUnwindContext) \ O(LeaveUnwindContext) \ O(ContinuePendingUnwind) \ diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 6e610433fd..aedbe50289 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -429,11 +429,6 @@ void PutByValue::execute_impl(Bytecode::Interpreter& interpreter) const } } -void LoadArgument::execute_impl(Bytecode::Interpreter& interpreter) const -{ - interpreter.accumulator() = interpreter.vm().argument(m_index); -} - void GetIterator::execute_impl(Bytecode::Interpreter& interpreter) const { interpreter.accumulator() = get_iterator(interpreter.global_object(), interpreter.accumulator()); @@ -672,11 +667,6 @@ String PutByValue::to_string_impl(const Bytecode::Executable&) const return String::formatted("PutByValue base:{}, property:{}", m_base, m_property); } -String LoadArgument::to_string_impl(const Bytecode::Executable&) const -{ - return String::formatted("LoadArgument {}", m_index); -} - String GetIterator::to_string_impl(Executable const&) const { return "GetIterator"; diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 055fdab701..85cb43d43f 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -651,22 +651,6 @@ private: HashMap m_variables; }; -class LoadArgument final : public Instruction { -public: - explicit LoadArgument(size_t index) - : Instruction(Type::LoadArgument) - , m_index(index) - { - } - - void execute_impl(Bytecode::Interpreter&) const; - String to_string_impl(Bytecode::Executable const&) const; - void replace_references_impl(BasicBlock const&, BasicBlock const&) { } - -private: - size_t m_index { 0 }; -}; - class GetIterator final : public Instruction { public: GetIterator() diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp index 3a3da65e36..81acec4917 100644 --- a/Userland/Libraries/LibJS/Parser.cpp +++ b/Userland/Libraries/LibJS/Parser.cpp @@ -659,23 +659,7 @@ Parser::PrimaryExpressionParseResult Parser::parse_primary_expression() set_try_parse_arrow_function_expression_failed_at_position(position(), true); } auto string = consume().value(); - Optional argument_index; - if (!m_state.function_parameters.is_empty()) { - size_t i = 0; - for (auto& parameter : m_state.function_parameters.last()) { - parameter.binding.visit( - [&](FlyString const& name) { - if (name == string) { - argument_index = i; - } - }, - [&](BindingPattern const&) { - }); - ++i; - } - } - argument_index = {}; - return { create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, string, argument_index) }; + return { create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, string) }; } case TokenType::NumericLiteral: return { create_ast_node({ m_state.current_token.filename(), rule_start.position(), position() }, consume_and_validate_numeric_literal().double_value()) }; diff --git a/Userland/Libraries/LibJS/Runtime/Reference.cpp b/Userland/Libraries/LibJS/Runtime/Reference.cpp index 76ccbb042e..2bc6b466f8 100644 --- a/Userland/Libraries/LibJS/Runtime/Reference.cpp +++ b/Userland/Libraries/LibJS/Runtime/Reference.cpp @@ -19,11 +19,6 @@ void Reference::put(GlobalObject& global_object, Value value) return; } - if (m_call_frame_argument_index.has_value()) { - global_object.vm().call_frame().arguments[m_call_frame_argument_index.value()] = value; - return; - } - if (is_local_variable() || is_global_variable()) { if (is_local_variable()) vm.set_variable(m_name.to_string(), value, global_object); @@ -73,9 +68,6 @@ Value Reference::get(GlobalObject& global_object) return {}; } - if (m_call_frame_argument_index.has_value()) - return global_object.vm().argument(m_call_frame_argument_index.value()); - if (is_local_variable() || is_global_variable()) { Value value; if (is_local_variable()) diff --git a/Userland/Libraries/LibJS/Runtime/Reference.h b/Userland/Libraries/LibJS/Runtime/Reference.h index 337d852623..864d7855c4 100644 --- a/Userland/Libraries/LibJS/Runtime/Reference.h +++ b/Userland/Libraries/LibJS/Runtime/Reference.h @@ -40,15 +40,6 @@ public: { } - enum CallFrameArgumentTag { CallFrameArgument }; - Reference(CallFrameArgumentTag, size_t index, FlyString const& name) - : m_base(js_null()) - , m_name(name) - , m_call_frame_argument_index(index) - , m_local_variable(true) - { - } - Value base() const { return m_base; } const PropertyName& name() const { return m_name; } bool is_strict() const { return m_strict; } @@ -83,7 +74,6 @@ private: Value m_base; PropertyName m_name; - Optional m_call_frame_argument_index; bool m_strict { false }; bool m_local_variable { false }; bool m_global_variable { false }; -- cgit v1.2.3