From aabd82d50886c14741500c03fc58fb3863b45ddb Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 22 Jun 2021 13:30:48 +0200 Subject: [PATCH] LibJS: Bring function environment records closer to the spec This patch adds FunctionEnvironmentRecord as a subclass of the existing DeclarativeEnvironmentRecord. Things that are specific to function environment records move into there, simplifying the base. Most of the abstract operations related to function environment records are rewritten to match the spec exactly. I also had to implement GetThisEnvironment() and GetSuperConstructor() to keep tests working after the changes, so that's nice as well. :^) --- Userland/Libraries/LibJS/AST.cpp | 18 ++-- Userland/Libraries/LibJS/CMakeLists.txt | 1 + Userland/Libraries/LibJS/Forward.h | 1 + Userland/Libraries/LibJS/Interpreter.cpp | 8 +- Userland/Libraries/LibJS/Interpreter.h | 2 +- .../LibJS/Runtime/AbstractOperations.cpp | 22 +++++ .../LibJS/Runtime/AbstractOperations.h | 2 + .../Libraries/LibJS/Runtime/BoundFunction.cpp | 2 +- .../Libraries/LibJS/Runtime/BoundFunction.h | 2 +- .../Runtime/DeclarativeEnvironmentRecord.cpp | 50 ----------- .../Runtime/DeclarativeEnvironmentRecord.h | 33 +------- .../LibJS/Runtime/EnvironmentRecord.h | 4 +- Userland/Libraries/LibJS/Runtime/Function.h | 7 +- .../Runtime/FunctionEnvironmentRecord.cpp | 83 +++++++++++++++++++ .../LibJS/Runtime/FunctionEnvironmentRecord.h | 66 +++++++++++++++ .../Libraries/LibJS/Runtime/GlobalObject.cpp | 10 --- .../Libraries/LibJS/Runtime/GlobalObject.h | 4 +- .../LibJS/Runtime/NativeFunction.cpp | 2 +- .../Libraries/LibJS/Runtime/NativeFunction.h | 2 +- Userland/Libraries/LibJS/Runtime/Object.h | 1 + .../LibJS/Runtime/ObjectEnvironmentRecord.cpp | 10 --- .../LibJS/Runtime/ObjectEnvironmentRecord.h | 2 - .../Libraries/LibJS/Runtime/ProxyObject.cpp | 2 +- .../Libraries/LibJS/Runtime/ProxyObject.h | 2 +- .../LibJS/Runtime/ScriptFunction.cpp | 11 +-- .../Libraries/LibJS/Runtime/ScriptFunction.h | 4 +- Userland/Libraries/LibJS/Runtime/VM.cpp | 32 +++---- Userland/Libraries/LibJS/Runtime/VM.h | 4 +- 28 files changed, 228 insertions(+), 159 deletions(-) create mode 100644 Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.cpp create mode 100644 Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.h diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 6f1f77011c..54322dad44 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -132,7 +133,7 @@ CallExpression::ThisAndCallee CallExpression::compute_this_and_callee(Interprete Object* this_value = nullptr; if (is(member_expression.object())) { - auto super_base = interpreter.current_declarative_environment_record()->get_super_base(); + auto super_base = interpreter.current_function_environment_record()->get_super_base(); if (super_base.is_nullish()) { vm.throw_exception(global_object, ErrorType::ObjectPrototypeNullOrUndefinedOnSuperPropertyAccess, super_base.to_string_without_side_effects()); return {}; @@ -227,14 +228,7 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj if (result.is_object()) new_object = &result.as_object(); } else if (is(*m_callee)) { - // FIXME: This is merely a band-aid to make super() inside catch {} work (which constructs - // a new DeclarativeEnvironmentRecord without current function). Implement GetSuperConstructor() - // and subsequently GetThisEnvironment() instead. - auto* function_environment = interpreter.current_declarative_environment_record(); - if (!function_environment->current_function()) - function_environment = static_cast(function_environment->outer_environment()); - - auto* super_constructor = function_environment->current_function()->prototype(); + auto* super_constructor = get_super_constructor(interpreter.vm()); // FIXME: Functions should track their constructor kind. if (!super_constructor || !super_constructor->is_function()) { vm.throw_exception(global_object, ErrorType::NotAConstructor, "Super constructor"); @@ -244,7 +238,9 @@ Value CallExpression::execute(Interpreter& interpreter, GlobalObject& global_obj if (vm.exception()) return {}; - function_environment->bind_this_value(global_object, result); + auto& this_er = get_this_environment(interpreter.vm()); + VERIFY(is(this_er)); + static_cast(this_er).bind_this_value(global_object, result); } else { result = vm.call(function, this_value, move(arguments)); } @@ -1355,7 +1351,7 @@ Value SpreadExpression::execute(Interpreter& interpreter, GlobalObject& global_o Value ThisExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const { InterpreterNodeScope node_scope { interpreter, *this }; - return interpreter.vm().resolve_this_binding(global_object); + return get_this_environment(interpreter.vm()).get_this_binding(global_object); } void ThisExpression::dump(int indent) const diff --git a/Userland/Libraries/LibJS/CMakeLists.txt b/Userland/Libraries/LibJS/CMakeLists.txt index 73c050d8bb..ff30f05704 100644 --- a/Userland/Libraries/LibJS/CMakeLists.txt +++ b/Userland/Libraries/LibJS/CMakeLists.txt @@ -61,6 +61,7 @@ set(SOURCES Runtime/FinalizationRegistryPrototype.cpp Runtime/FunctionConstructor.cpp Runtime/Function.cpp + Runtime/FunctionEnvironmentRecord.cpp Runtime/FunctionPrototype.cpp Runtime/GeneratorFunctionConstructor.cpp Runtime/GeneratorFunctionPrototype.cpp diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index 9f834e3a78..3af5edad7e 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -127,6 +127,7 @@ class Error; class ErrorType; class Exception; class Expression; +class FunctionEnvironmentRecord; class FunctionNode; class GlobalObject; class HandleImpl; diff --git a/Userland/Libraries/LibJS/Interpreter.cpp b/Userland/Libraries/LibJS/Interpreter.cpp index bc65abdf3c..8779ad23f7 100644 --- a/Userland/Libraries/LibJS/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Interpreter.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -193,10 +193,10 @@ Value Interpreter::execute_statement(GlobalObject& global_object, const Statemen return last_value; } -DeclarativeEnvironmentRecord* Interpreter::current_declarative_environment_record() +FunctionEnvironmentRecord* Interpreter::current_function_environment_record() { - VERIFY(is(vm().call_frame().environment_record)); - return static_cast(vm().call_frame().environment_record); + VERIFY(is(vm().call_frame().environment_record)); + return static_cast(vm().call_frame().environment_record); } } diff --git a/Userland/Libraries/LibJS/Interpreter.h b/Userland/Libraries/LibJS/Interpreter.h index 1a0adf0495..182699cd45 100644 --- a/Userland/Libraries/LibJS/Interpreter.h +++ b/Userland/Libraries/LibJS/Interpreter.h @@ -58,7 +58,7 @@ public: EnvironmentRecord* current_environment_record() { return vm().current_environment_record(); } - DeclarativeEnvironmentRecord* current_declarative_environment_record(); + FunctionEnvironmentRecord* current_function_environment_record(); void enter_scope(const ScopeNode&, ScopeType, GlobalObject&); void exit_scope(const ScopeNode&); diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp index 52ee895ee4..6fbfb754c4 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -176,4 +177,25 @@ ObjectEnvironmentRecord* new_object_environment(Object& object, bool is_with_env return global_object.heap().allocate(global_object, object, environment_record); } +// 9.4.3 GetThisEnvironment ( ), https://tc39.es/ecma262/#sec-getthisenvironment +EnvironmentRecord& get_this_environment(VM& vm) +{ + // FIXME: Should be the *lexical* environment. + for (auto* env = vm.current_environment_record(); env; env = env->outer_environment()) { + if (env->has_this_binding()) + return *env; + } + VERIFY_NOT_REACHED(); +} + +// 13.3.7.2 GetSuperConstructor ( ), https://tc39.es/ecma262/#sec-getsuperconstructor +Object* get_super_constructor(VM& vm) +{ + auto& env = get_this_environment(vm); + VERIFY(is(env)); + auto& active_function = static_cast(env).function_object(); + auto* super_constructor = active_function.prototype(); + return super_constructor; +} + } diff --git a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h index 0b4a7b22cd..737ebdf368 100644 --- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.h +++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.h @@ -15,6 +15,8 @@ namespace JS { DeclarativeEnvironmentRecord* new_declarative_environment(EnvironmentRecord&); ObjectEnvironmentRecord* new_object_environment(Object&, bool is_with_environment, EnvironmentRecord*); +EnvironmentRecord& get_this_environment(VM&); +Object* get_super_constructor(VM&); Value require_object_coercible(GlobalObject&, Value); Function* get_method(GlobalObject& global_object, Value, PropertyName const&); size_t length_of_array_like(GlobalObject&, Object const&); diff --git a/Userland/Libraries/LibJS/Runtime/BoundFunction.cpp b/Userland/Libraries/LibJS/Runtime/BoundFunction.cpp index ae108f6f5c..73f7751f1c 100644 --- a/Userland/Libraries/LibJS/Runtime/BoundFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/BoundFunction.cpp @@ -44,7 +44,7 @@ Value BoundFunction::construct(Function& new_target) return m_target_function->construct(new_target); } -DeclarativeEnvironmentRecord* BoundFunction::create_environment_record() +FunctionEnvironmentRecord* BoundFunction::create_environment_record() { return m_target_function->create_environment_record(); } diff --git a/Userland/Libraries/LibJS/Runtime/BoundFunction.h b/Userland/Libraries/LibJS/Runtime/BoundFunction.h index 165d73f9de..bdee8f5195 100644 --- a/Userland/Libraries/LibJS/Runtime/BoundFunction.h +++ b/Userland/Libraries/LibJS/Runtime/BoundFunction.h @@ -22,7 +22,7 @@ public: virtual Value construct(Function& new_target) override; - virtual DeclarativeEnvironmentRecord* create_environment_record() override; + virtual FunctionEnvironmentRecord* create_environment_record() override; virtual void visit_edges(Visitor&) override; diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.cpp index 5a8f8ef91a..7b49451adb 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.cpp +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.cpp @@ -49,9 +49,6 @@ DeclarativeEnvironmentRecord::~DeclarativeEnvironmentRecord() void DeclarativeEnvironmentRecord::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_this_value); - visitor.visit(m_new_target); - visitor.visit(m_current_function); for (auto& it : m_variables) visitor.visit(it.value.value); } @@ -71,51 +68,4 @@ bool DeclarativeEnvironmentRecord::delete_from_environment_record(FlyString cons return m_variables.remove(name); } -Value DeclarativeEnvironmentRecord::get_super_base() -{ - if (m_environment_record_type != EnvironmentRecordType::Function) - return {}; - VERIFY(m_current_function); - auto home_object = m_current_function->home_object(); - if (!home_object.is_object()) - return {}; - return home_object.as_object().prototype(); -} - -bool DeclarativeEnvironmentRecord::has_this_binding() const -{ - // More like "is_capable_of_having_a_this_binding". - switch (m_environment_record_type) { - case EnvironmentRecordType::Declarative: - case EnvironmentRecordType::Object: - return false; - case EnvironmentRecordType::Function: - return this_binding_status() != ThisBindingStatus::Lexical; - case EnvironmentRecordType::Module: - return true; - } - VERIFY_NOT_REACHED(); -} - -Value DeclarativeEnvironmentRecord::get_this_binding(GlobalObject& global_object) const -{ - VERIFY(has_this_binding()); - if (this_binding_status() == ThisBindingStatus::Uninitialized) { - vm().throw_exception(global_object, ErrorType::ThisHasNotBeenInitialized); - return {}; - } - return m_this_value; -} - -void DeclarativeEnvironmentRecord::bind_this_value(GlobalObject& global_object, Value this_value) -{ - VERIFY(has_this_binding()); - if (m_this_binding_status == ThisBindingStatus::Initialized) { - vm().throw_exception(global_object, ErrorType::ThisIsAlreadyInitialized); - return; - } - m_this_value = this_value; - m_this_binding_status = ThisBindingStatus::Initialized; -} - } diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.h index baac4a26a1..6bd7af3727 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironmentRecord.h @@ -13,16 +13,10 @@ namespace JS { -class DeclarativeEnvironmentRecord final : public EnvironmentRecord { +class DeclarativeEnvironmentRecord : public EnvironmentRecord { JS_OBJECT(DeclarativeEnvironmentRecord, EnvironmentRecord); public: - enum class ThisBindingStatus { - Lexical, - Initialized, - Uninitialized, - }; - enum class EnvironmentRecordType { Declarative, Function, @@ -41,38 +35,19 @@ public: virtual Optional get_from_environment_record(FlyString const&) const override; virtual void put_into_environment_record(FlyString const&, Variable) override; virtual bool delete_from_environment_record(FlyString const&) override; - virtual bool has_this_binding() const override; - virtual Value get_this_binding(GlobalObject&) const override; HashMap const& variables() const { return m_variables; } - Value get_super_base(); - - ThisBindingStatus this_binding_status() const { return m_this_binding_status; } - void bind_this_value(GlobalObject&, Value this_value); - - // Not a standard operation. - void replace_this_binding(Value this_value) { m_this_value = this_value; } - - Value new_target() const { return m_new_target; }; - void set_new_target(Value new_target) { m_new_target = new_target; } - - Function* current_function() const { return m_current_function; } - void set_current_function(Function& function) { m_current_function = &function; } - EnvironmentRecordType type() const { return m_environment_record_type; } +protected: + virtual void visit_edges(Visitor&) override; + private: virtual bool is_declarative_environment_record() const override { return true; } - virtual void visit_edges(Visitor&) override; EnvironmentRecordType m_environment_record_type : 8 { EnvironmentRecordType::Declarative }; - ThisBindingStatus m_this_binding_status : 8 { ThisBindingStatus::Uninitialized }; HashMap m_variables; - Value m_this_value; - Value m_new_target; - // Corresponds to [[FunctionObject]] - Function* m_current_function { nullptr }; }; template<> diff --git a/Userland/Libraries/LibJS/Runtime/EnvironmentRecord.h b/Userland/Libraries/LibJS/Runtime/EnvironmentRecord.h index 8f4ce18c55..c0eecf6dca 100644 --- a/Userland/Libraries/LibJS/Runtime/EnvironmentRecord.h +++ b/Userland/Libraries/LibJS/Runtime/EnvironmentRecord.h @@ -23,8 +23,8 @@ public: virtual void put_into_environment_record(FlyString const&, Variable) = 0; virtual bool delete_from_environment_record(FlyString const&) = 0; - virtual bool has_this_binding() const = 0; - virtual Value get_this_binding(GlobalObject&) const = 0; + virtual bool has_this_binding() const { return false; } + virtual Value get_this_binding(GlobalObject&) const { return {}; } // [[OuterEnv]] EnvironmentRecord* outer_environment() { return m_outer_environment; } diff --git a/Userland/Libraries/LibJS/Runtime/Function.h b/Userland/Libraries/LibJS/Runtime/Function.h index 21b9cfa9c7..aae2e11e79 100644 --- a/Userland/Libraries/LibJS/Runtime/Function.h +++ b/Userland/Libraries/LibJS/Runtime/Function.h @@ -26,7 +26,7 @@ public: virtual Value call() = 0; virtual Value construct(Function& new_target) = 0; virtual const FlyString& name() const = 0; - virtual DeclarativeEnvironmentRecord* create_environment_record() = 0; + virtual FunctionEnvironmentRecord* create_environment_record() = 0; BoundFunction* bind(Value bound_this_value, Vector arguments); @@ -42,6 +42,11 @@ public: virtual bool is_strict_mode() const { return false; } + // [[Environment]] + // The Environment Record that the function was closed over. + // Used as the outer environment when evaluating the code of the function. + virtual EnvironmentRecord* environment() { return nullptr; } + protected: virtual void visit_edges(Visitor&) override; diff --git a/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.cpp b/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.cpp new file mode 100644 index 0000000000..4047bd9185 --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.cpp @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2021, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +namespace JS { + +FunctionEnvironmentRecord::FunctionEnvironmentRecord(EnvironmentRecord* parent_scope, HashMap variables) + : DeclarativeEnvironmentRecord(variables, parent_scope, DeclarativeEnvironmentRecord::EnvironmentRecordType::Function) +{ +} + +FunctionEnvironmentRecord::~FunctionEnvironmentRecord() +{ +} + +void FunctionEnvironmentRecord::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_this_value); + visitor.visit(m_new_target); + visitor.visit(m_function_object); +} + +// 9.1.1.3.5 GetSuperBase ( ), https://tc39.es/ecma262/#sec-getsuperbase +Value FunctionEnvironmentRecord::get_super_base() const +{ + VERIFY(m_function_object); + auto home_object = m_function_object->home_object(); + if (home_object.is_undefined()) + return js_undefined(); + return home_object.as_object().prototype(); +} + +// 9.1.1.3.2 HasThisBinding ( ), https://tc39.es/ecma262/#sec-function-environment-records-hasthisbinding +bool FunctionEnvironmentRecord::has_this_binding() const +{ + if (this_binding_status() == ThisBindingStatus::Lexical) + return false; + return true; +} + +// 9.1.1.3.3 HasSuperBinding ( ), https://tc39.es/ecma262/#sec-function-environment-records-hassuperbinding +bool FunctionEnvironmentRecord::has_super_binding() const +{ + if (this_binding_status() == ThisBindingStatus::Lexical) + return false; + if (function_object().home_object().is_undefined()) + return false; + return true; +} + +// 9.1.1.3.4 GetThisBinding ( ), https://tc39.es/ecma262/#sec-function-environment-records-getthisbinding +Value FunctionEnvironmentRecord::get_this_binding(GlobalObject& global_object) const +{ + VERIFY(has_this_binding()); + if (this_binding_status() == ThisBindingStatus::Uninitialized) { + vm().throw_exception(global_object, ErrorType::ThisHasNotBeenInitialized); + return {}; + } + return m_this_value; +} + +// 9.1.1.3.1 BindThisValue ( V ), https://tc39.es/ecma262/#sec-bindthisvalue +Value FunctionEnvironmentRecord::bind_this_value(GlobalObject& global_object, Value this_value) +{ + VERIFY(has_this_binding()); + if (this_binding_status() == ThisBindingStatus::Initialized) { + vm().throw_exception(global_object, ErrorType::ThisIsAlreadyInitialized); + return {}; + } + m_this_value = this_value; + m_this_binding_status = ThisBindingStatus::Initialized; + return this_value; +} + +} diff --git a/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.h b/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.h new file mode 100644 index 0000000000..fe66b3c6da --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/FunctionEnvironmentRecord.h @@ -0,0 +1,66 @@ +/* + * Copyright (c) 2021, Andreas Kling + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace JS { + +class FunctionEnvironmentRecord final : public DeclarativeEnvironmentRecord { + JS_OBJECT(FunctionEnvironmentRecord, DeclarativeEnvironmentRecord); + +public: + enum class ThisBindingStatus : u8 { + Lexical, + Initialized, + Uninitialized, + }; + + FunctionEnvironmentRecord(EnvironmentRecord* parent_scope, HashMap variables); + virtual ~FunctionEnvironmentRecord() override; + + // [[ThisValue]] + Value this_value() const { return m_this_value; } + void set_this_value(Value value) { m_this_value = value; } + + // Not a standard operation. + void replace_this_binding(Value this_value) { m_this_value = this_value; } + + // [[ThisBindingStatus]] + ThisBindingStatus this_binding_status() const { return m_this_binding_status; } + void set_this_binding_status(ThisBindingStatus status) { m_this_binding_status = status; } + + // [[FunctionObject]] + Function& function_object() { return *m_function_object; } + Function const& function_object() const { return *m_function_object; } + void set_function_object(Function& function) { m_function_object = &function; } + + // [[NewTarget]] + Value new_target() const { return m_new_target; } + void set_new_target(Value new_target) { m_new_target = new_target; } + + // Abstract operations + Value get_super_base() const; + bool has_super_binding() const; + virtual bool has_this_binding() const override; + virtual Value get_this_binding(GlobalObject&) const override; + Value bind_this_value(GlobalObject&, Value); + +private: + virtual bool is_function_environment_record() const override { return true; } + virtual void visit_edges(Visitor&) override; + + Value m_this_value; + ThisBindingStatus m_this_binding_status { ThisBindingStatus::Uninitialized }; + Function* m_function_object { nullptr }; + Value m_new_target; +}; + +template<> +inline bool Object::fast_is() const { return is_function_environment_record(); } + +} diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index aada03b332..173d23d91d 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -346,16 +346,6 @@ bool GlobalObject::delete_from_environment_record(FlyString const& name) return delete_property(name); } -bool GlobalObject::has_this_binding() const -{ - return true; -} - -Value GlobalObject::get_this_binding(GlobalObject&) const -{ - return Value(this); -} - // 19.2.1 eval ( x ), https://tc39.es/ecma262/#sec-eval-x JS_DEFINE_NATIVE_FUNCTION(GlobalObject::eval) { diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.h b/Userland/Libraries/LibJS/Runtime/GlobalObject.h index a96e9bc550..b59254dac1 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.h +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.h @@ -24,8 +24,8 @@ public: virtual Optional get_from_environment_record(FlyString const&) const override; virtual void put_into_environment_record(FlyString const&, Variable) override; virtual bool delete_from_environment_record(FlyString const&) override; - virtual bool has_this_binding() const override; - virtual Value get_this_binding(GlobalObject&) const override; + virtual bool has_this_binding() const final { return true; } + virtual Value get_this_binding(GlobalObject&) const final { return this; } Console& console() { return *m_console; } diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp index cc4634c6cf..f0e711fd9a 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp @@ -47,7 +47,7 @@ Value NativeFunction::construct(Function&) return {}; } -DeclarativeEnvironmentRecord* NativeFunction::create_environment_record() +FunctionEnvironmentRecord* NativeFunction::create_environment_record() { return nullptr; } diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.h b/Userland/Libraries/LibJS/Runtime/NativeFunction.h index 3586545e08..41a5b6ac3e 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.h +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.h @@ -34,7 +34,7 @@ protected: explicit NativeFunction(Object& prototype); private: - virtual DeclarativeEnvironmentRecord* create_environment_record() override final; + virtual FunctionEnvironmentRecord* create_environment_record() override final; virtual bool is_native_function() const final { return true; } FlyString m_name; diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index fb9b781bb2..4fbc23de43 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -109,6 +109,7 @@ public: virtual bool is_proxy_object() const { return false; } virtual bool is_native_function() const { return false; } virtual bool is_declarative_environment_record() const { return false; } + virtual bool is_function_environment_record() const { return false; } // B.3.7 The [[IsHTMLDDA]] Internal Slot, https://tc39.es/ecma262/#sec-IsHTMLDDA-internal-slot virtual bool is_htmldda() const { return false; } diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.cpp index fae58a56f0..69adf9e8d1 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.cpp +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.cpp @@ -39,14 +39,4 @@ bool ObjectEnvironmentRecord::delete_from_environment_record(FlyString const& na return m_object.delete_property(name); } -bool ObjectEnvironmentRecord::has_this_binding() const -{ - return outer_environment()->has_this_binding(); -} - -Value ObjectEnvironmentRecord::get_this_binding(GlobalObject& global_object) const -{ - return outer_environment()->get_this_binding(global_object); -} - } diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.h index d19c802691..c6c2bf1f32 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.h +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironmentRecord.h @@ -19,8 +19,6 @@ public: virtual Optional get_from_environment_record(FlyString const&) const override; virtual void put_into_environment_record(FlyString const&, Variable) override; virtual bool delete_from_environment_record(FlyString const&) override; - virtual bool has_this_binding() const override; - virtual Value get_this_binding(GlobalObject&) const override; private: virtual void visit_edges(Visitor&) override; diff --git a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp index 1b6e912942..a1910c8c65 100644 --- a/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ProxyObject.cpp @@ -478,7 +478,7 @@ const FlyString& ProxyObject::name() const return static_cast(m_target).name(); } -DeclarativeEnvironmentRecord* ProxyObject::create_environment_record() +FunctionEnvironmentRecord* ProxyObject::create_environment_record() { VERIFY(is_function()); return static_cast(m_target).create_environment_record(); diff --git a/Userland/Libraries/LibJS/Runtime/ProxyObject.h b/Userland/Libraries/LibJS/Runtime/ProxyObject.h index 8493a444cf..cd97d87548 100644 --- a/Userland/Libraries/LibJS/Runtime/ProxyObject.h +++ b/Userland/Libraries/LibJS/Runtime/ProxyObject.h @@ -22,7 +22,7 @@ public: virtual Value call() override; virtual Value construct(Function& new_target) override; virtual const FlyString& name() const override; - virtual DeclarativeEnvironmentRecord* create_environment_record() override; + virtual FunctionEnvironmentRecord* create_environment_record() override; const Object& target() const { return m_target; } const Object& handler() const { return m_handler; } diff --git a/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp b/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp index 57133f7db2..47cd3e8f3b 100644 --- a/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/ScriptFunction.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -93,7 +94,7 @@ void ScriptFunction::visit_edges(Visitor& visitor) visitor.visit(m_parent_scope); } -DeclarativeEnvironmentRecord* ScriptFunction::create_environment_record() +FunctionEnvironmentRecord* ScriptFunction::create_environment_record() { HashMap variables; for (auto& parameter : m_parameters) { @@ -122,11 +123,11 @@ DeclarativeEnvironmentRecord* ScriptFunction::create_environment_record() } } - auto* environment = heap().allocate(global_object(), move(variables), m_parent_scope, DeclarativeEnvironmentRecord::EnvironmentRecordType::Function); - environment->set_current_function(*this); + auto* environment = heap().allocate(global_object(), m_parent_scope, variables); + environment->set_function_object(*this); if (m_is_arrow_function) { - if (is(m_parent_scope)) - environment->set_new_target(static_cast(m_parent_scope)->new_target()); + if (is(m_parent_scope)) + environment->set_new_target(static_cast(m_parent_scope)->new_target()); } return environment; } diff --git a/Userland/Libraries/LibJS/Runtime/ScriptFunction.h b/Userland/Libraries/LibJS/Runtime/ScriptFunction.h index ab558c044d..8c409605f4 100644 --- a/Userland/Libraries/LibJS/Runtime/ScriptFunction.h +++ b/Userland/Libraries/LibJS/Runtime/ScriptFunction.h @@ -35,11 +35,13 @@ public: auto& bytecode_executable() const { return m_bytecode_executable; } + virtual EnvironmentRecord* environment() override { return m_parent_scope; } + protected: virtual bool is_strict_mode() const final { return m_is_strict; } private: - virtual DeclarativeEnvironmentRecord* create_environment_record() override; + virtual FunctionEnvironmentRecord* create_environment_record() override; virtual void visit_edges(Visitor&) override; Value execute_function_body(); diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 71026c58f0..a313801049 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2020-2021, Andreas Kling * Copyright (c) 2020-2021, Linus Groh * * SPDX-License-Identifier: BSD-2-Clause @@ -9,9 +9,11 @@ #include #include #include +#include #include #include #include +#include #include #include #include @@ -460,8 +462,8 @@ Value VM::construct(Function& function, Function& new_target, Optional(current_environment_record())); - static_cast(current_environment_record())->replace_this_binding(result); + VERIFY(is(current_environment_record())); + static_cast(current_environment_record())->replace_this_binding(result); } auto prototype = new_target.get(names.prototype); if (exception()) @@ -500,25 +502,11 @@ String VM::join_arguments(size_t start_index) const return joined_arguments.build(); } -Value VM::resolve_this_binding(GlobalObject& global_object) const +Value VM::get_new_target() { - return find_this_scope()->get_this_binding(global_object); -} - -const EnvironmentRecord* VM::find_this_scope() const -{ - // We will always return because the Global environment will always be reached, which has a |this| binding. - for (auto* environment_record = current_environment_record(); environment_record; environment_record = environment_record->outer_environment()) { - if (environment_record->has_this_binding()) - return environment_record; - } - VERIFY_NOT_REACHED(); -} - -Value VM::get_new_target() const -{ - VERIFY(is(find_this_scope())); - return static_cast(find_this_scope())->new_target(); + auto& env = get_this_environment(*this); + VERIFY(is(env)); + return static_cast(env).new_target(); } Value VM::call_internal(Function& function, Value this_value, Optional arguments) @@ -540,7 +528,7 @@ Value VM::call_internal(Function& function, Value this_value, Optionalthis_binding_status() == DeclarativeEnvironmentRecord::ThisBindingStatus::Uninitialized); + VERIFY(environment->this_binding_status() == FunctionEnvironmentRecord::ThisBindingStatus::Uninitialized); environment->bind_this_value(function.global_object(), call_frame.this_value); } diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index 8defd28d5a..c095541f56 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -222,9 +222,7 @@ public: String join_arguments(size_t start_index = 0) const; - Value resolve_this_binding(GlobalObject&) const; - const EnvironmentRecord* find_this_scope() const; - Value get_new_target() const; + Value get_new_target(); template [[nodiscard]] ALWAYS_INLINE Value call(Function& function, Value this_value, Args... args)