From 2484bbc4e03744f3b9a17649d81b6e5952fd348b Mon Sep 17 00:00:00 2001 From: davidot Date: Thu, 1 Sep 2022 23:22:17 +0200 Subject: [PATCH] LibJS: Make DeclarativeEnvironment based on bindings instead of indices This will allow other environments which extend DeclarativeEnvironment to provide their own custom bindings. --- .../LibJS/Runtime/DeclarativeEnvironment.cpp | 61 ++++++++++--------- .../LibJS/Runtime/DeclarativeEnvironment.h | 47 ++++++++++++-- 2 files changed, 73 insertions(+), 35 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp index 616143ffae..d910d0a61c 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp @@ -47,11 +47,11 @@ void DeclarativeEnvironment::visit_edges(Visitor& visitor) // 9.1.1.1.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-declarative-environment-records-hasbinding-n ThrowCompletionOr DeclarativeEnvironment::has_binding(FlyString const& name, Optional* out_index) const { - auto index = find_binding_index(name); - if (!index.has_value()) + auto binding_and_index = find_binding_and_index(name); + if (!binding_and_index.has_value()) return false; - if (!is_permanently_screwed_by_eval() && out_index) - *out_index = *index; + if (!is_permanently_screwed_by_eval() && out_index && binding_and_index->index().has_value()) + *out_index = *(binding_and_index->index()); return true; } @@ -98,16 +98,14 @@ ThrowCompletionOr DeclarativeEnvironment::create_immutable_binding(VM&, Fl // 9.1.1.1.4 InitializeBinding ( N, V ), https://tc39.es/ecma262/#sec-declarative-environment-records-initializebinding-n-v ThrowCompletionOr DeclarativeEnvironment::initialize_binding(VM& vm, FlyString const& name, Value value) { - auto index = find_binding_index(name); - VERIFY(index.has_value()); + auto binding_and_index = find_binding_and_index(name); + VERIFY(binding_and_index.has_value()); - return initialize_binding_direct(vm, *index, value); + return initialize_binding_direct(vm, binding_and_index->binding(), value); } -ThrowCompletionOr DeclarativeEnvironment::initialize_binding_direct(VM&, size_t index, Value value) +ThrowCompletionOr DeclarativeEnvironment::initialize_binding_direct(VM&, Binding& binding, Value value) { - auto& binding = m_bindings[index]; - // 1. Assert: envRec must have an uninitialized binding for N. VERIFY(binding.initialized == false); @@ -125,8 +123,8 @@ ThrowCompletionOr DeclarativeEnvironment::initialize_binding_direct(VM&, s ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding(VM& vm, FlyString const& name, Value value, bool strict) { // 1. If envRec does not have a binding for N, then - auto index = find_binding_index(name); - if (!index.has_value()) { + auto binding_and_index = find_binding_and_index(name); + if (!binding_and_index.has_value()) { // a. If S is true, throw a ReferenceError exception. if (strict) return vm.throw_completion(ErrorType::UnknownIdentifier, name); @@ -142,7 +140,7 @@ ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding(VM& vm, FlyS } // 2-5. (extracted into a non-standard function below) - TRY(set_mutable_binding_direct(vm, *index, value, strict)); + TRY(set_mutable_binding_direct(vm, binding_and_index->binding(), value, strict)); // 6. Return unused. return {}; @@ -150,7 +148,11 @@ ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding(VM& vm, FlyS ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding_direct(VM& vm, size_t index, Value value, bool strict) { - auto& binding = m_bindings[index]; + return set_mutable_binding_direct(vm, m_bindings[index], value, strict); +} + +ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding_direct(VM& vm, Binding& binding, Value value, bool strict) +{ if (binding.strict) strict = true; @@ -171,17 +173,20 @@ ThrowCompletionOr DeclarativeEnvironment::set_mutable_binding_direct(VM& v ThrowCompletionOr DeclarativeEnvironment::get_binding_value(VM& vm, FlyString const& name, bool strict) { // 1. Assert: envRec has a binding for N. - auto index = find_binding_index(name); - VERIFY(index.has_value()); + auto binding_and_index = find_binding_and_index(name); + VERIFY(binding_and_index.has_value()); // 2-3. (extracted into a non-standard function below) - return get_binding_value_direct(vm, *index, strict); + return get_binding_value_direct(vm, binding_and_index->binding(), strict); } -ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(VM&, size_t index, bool) +ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(VM& vm, size_t index, bool strict) { - auto& binding = m_bindings[index]; + return get_binding_value_direct(vm, m_bindings[index], strict); +} +ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(VM&, Binding& binding, bool) +{ // 2. If the binding for N in envRec is an uninitialized binding, throw a ReferenceError exception. if (!binding.initialized) return vm().throw_completion(ErrorType::BindingNotInitialized, binding.name); @@ -194,18 +199,16 @@ ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(VM&, s ThrowCompletionOr DeclarativeEnvironment::delete_binding(VM&, FlyString const& name) { // 1. Assert: envRec has a binding for the name that is the value of N. - auto index = find_binding_index(name); - VERIFY(index.has_value()); - - auto& binding = m_bindings[*index]; + auto binding_and_index = find_binding_and_index(name); + VERIFY(binding_and_index.has_value()); // 2. If the binding for N in envRec cannot be deleted, return false. - if (!binding.can_be_deleted) + if (!binding_and_index->binding().can_be_deleted) return false; // 3. Remove the binding for N from envRec. // NOTE: We keep the entries in m_bindings to avoid disturbing indices. - binding = {}; + binding_and_index->binding() = {}; // 4. Return true. return true; @@ -213,10 +216,10 @@ ThrowCompletionOr DeclarativeEnvironment::delete_binding(VM&, FlyString co ThrowCompletionOr DeclarativeEnvironment::initialize_or_set_mutable_binding(VM& vm, FlyString const& name, Value value) { - auto index = find_binding_index(name); - VERIFY(index.has_value()); - auto& binding = m_bindings[*index]; - if (!binding.initialized) + auto binding_and_index = find_binding_and_index(name); + VERIFY(binding_and_index.has_value()); + + if (!binding_and_index->binding().initialized) TRY(initialize_binding(vm, name, value)); else TRY(set_mutable_binding(vm, name, value, false)); diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h index 0110f57d59..ceea57c9d7 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h @@ -54,9 +54,13 @@ public: return names; } - ThrowCompletionOr initialize_binding_direct(VM&, size_t index, Value); - ThrowCompletionOr get_binding_value_direct(VM&, size_t index, bool strict); ThrowCompletionOr set_mutable_binding_direct(VM&, size_t index, Value, bool strict); + ThrowCompletionOr get_binding_value_direct(VM&, size_t index, bool strict); + +private: + ThrowCompletionOr initialize_binding_direct(VM&, Binding&, Value); + ThrowCompletionOr get_binding_value_direct(VM&, Binding&, bool strict); + ThrowCompletionOr set_mutable_binding_direct(VM&, Binding&, Value, bool strict); protected: DeclarativeEnvironment(); @@ -65,10 +69,37 @@ protected: virtual void visit_edges(Visitor&) override; -private: - virtual bool is_declarative_environment() const override { return true; } + class BindingAndIndex { + public: + Binding& binding() + { + if (m_referenced_binding) + return *m_referenced_binding; + return m_temporary_binding; + } - Optional find_binding_index(FlyString const& name) const + BindingAndIndex(Binding* binding, Optional index) + : m_referenced_binding(binding) + , m_index(move(index)) + { + } + + explicit BindingAndIndex(Binding temporary_binding) + : m_temporary_binding(move(temporary_binding)) + { + } + + Optional const& index() const { return m_index; } + + private: + Binding* m_referenced_binding { nullptr }; + Binding m_temporary_binding {}; + Optional m_index; + }; + + friend class ModuleEnvironment; + + virtual Optional find_binding_and_index(FlyString const& name) const { auto it = m_bindings.find_if([&](auto const& binding) { return binding.name == name; @@ -76,9 +107,13 @@ private: if (it == m_bindings.end()) return {}; - return it.index(); + + return BindingAndIndex { const_cast(&(*it)), it.index() }; } +private: + virtual bool is_declarative_environment() const override { return true; } + Vector m_bindings; };