From 01370136eebfaa9e1126b9b70ff5232f7dfb3be7 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 9 Oct 2021 19:49:08 +0100 Subject: [PATCH] LibJS: Convert delete_binding() to ThrowCompletionOr Also add spec step comments to it while we're here. --- .../LibJS/Runtime/DeclarativeEnvironment.cpp | 10 ++++++- .../LibJS/Runtime/DeclarativeEnvironment.h | 2 +- .../Libraries/LibJS/Runtime/Environment.h | 2 +- .../LibJS/Runtime/GlobalEnvironment.cpp | 27 ++++++++++++++++--- .../LibJS/Runtime/GlobalEnvironment.h | 2 +- .../LibJS/Runtime/ObjectEnvironment.cpp | 6 +++-- .../LibJS/Runtime/ObjectEnvironment.h | 2 +- .../Libraries/LibJS/Runtime/Reference.cpp | 2 +- 8 files changed, 41 insertions(+), 12 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp index 97d593d989d..214f1a28c66 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp @@ -175,16 +175,24 @@ ThrowCompletionOr DeclarativeEnvironment::get_binding_value_direct(Global } // 9.1.1.1.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-declarative-environment-records-deletebinding-n -bool DeclarativeEnvironment::delete_binding(GlobalObject&, FlyString const& name) +ThrowCompletionOr DeclarativeEnvironment::delete_binding(GlobalObject&, FlyString const& name) { + // 1. Assert: envRec has a binding for the name that is the value of N. auto it = m_names.find(name); VERIFY(it != m_names.end()); + auto& binding = m_bindings[it->value]; + + // 2. If the binding for N in envRec cannot be deleted, return false. if (!binding.can_be_deleted) return false; + + // 3. Remove the binding for N from envRec. // NOTE: We keep the entry in m_bindings to avoid disturbing indices. binding = {}; m_names.remove(it); + + // 4. Return true. return true; } diff --git a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h index c843663d49b..934c14400d1 100644 --- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h @@ -28,7 +28,7 @@ public: virtual ThrowCompletionOr initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual ThrowCompletionOr set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual ThrowCompletionOr get_binding_value(GlobalObject&, FlyString const& name, bool strict) override; - virtual bool delete_binding(GlobalObject&, FlyString const& name) override; + virtual ThrowCompletionOr delete_binding(GlobalObject&, FlyString const& name) override; void initialize_or_set_mutable_binding(Badge, GlobalObject& global_object, FlyString const& name, Value value); diff --git a/Userland/Libraries/LibJS/Runtime/Environment.h b/Userland/Libraries/LibJS/Runtime/Environment.h index b4fdd07f18d..ec94a3b1e39 100644 --- a/Userland/Libraries/LibJS/Runtime/Environment.h +++ b/Userland/Libraries/LibJS/Runtime/Environment.h @@ -39,7 +39,7 @@ public: virtual ThrowCompletionOr initialize_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value) { return {}; } virtual ThrowCompletionOr set_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value, [[maybe_unused]] bool strict) { return {}; } virtual ThrowCompletionOr get_binding_value(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return Value {}; } - virtual bool delete_binding(GlobalObject&, [[maybe_unused]] FlyString const& name) { return false; } + virtual ThrowCompletionOr delete_binding(GlobalObject&, [[maybe_unused]] FlyString const& name) { return false; } // [[OuterEnv]] Environment* outer_environment() { return m_outer_environment; } diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp index 598c6df87e2..d2dbf2bb259 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp @@ -122,19 +122,38 @@ ThrowCompletionOr GlobalEnvironment::get_binding_value(GlobalObject& glob } // 9.1.1.4.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-global-environment-records-deletebinding-n -bool GlobalEnvironment::delete_binding(GlobalObject& global_object, FlyString const& name) +ThrowCompletionOr GlobalEnvironment::delete_binding(GlobalObject& global_object, FlyString const& name) { - if (MUST(m_declarative_record->has_binding(name))) + // 1. Let DclRec be envRec.[[DeclarativeRecord]]. + // 2. If DclRec.HasBinding(N) is true, then + if (MUST(m_declarative_record->has_binding(name))) { + // a. Return DclRec.DeleteBinding(N). return m_declarative_record->delete_binding(global_object, name); + } - bool existing_prop = TRY_OR_DISCARD(m_object_record->binding_object().has_own_property(name)); + // 3. Let ObjRec be envRec.[[ObjectRecord]]. + // 4. Let globalObject be ObjRec.[[BindingObject]]. + + // 5. Let existingProp be ? HasOwnProperty(globalObject, N). + bool existing_prop = TRY(m_object_record->binding_object().has_own_property(name)); + + // 6. If existingProp is true, then if (existing_prop) { - bool status = m_object_record->delete_binding(global_object, name); + // a. Let status be ? ObjRec.DeleteBinding(N). + bool status = TRY(m_object_record->delete_binding(global_object, name)); + + // b. If status is true, then if (status) { + // i. Let varNames be envRec.[[VarNames]]. + // ii. If N is an element of varNames, remove that element from the varNames. m_var_names.remove_all_matching([&](auto& entry) { return entry == name; }); } + + // c. Return status. return status; } + + // 7. Return true. return true; } diff --git a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h index 27863b01784..8379fb326c6 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h @@ -25,7 +25,7 @@ public: virtual ThrowCompletionOr initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual ThrowCompletionOr set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual ThrowCompletionOr get_binding_value(GlobalObject&, FlyString const& name, bool strict) override; - virtual bool delete_binding(GlobalObject&, FlyString const& name) override; + virtual ThrowCompletionOr delete_binding(GlobalObject&, FlyString const& name) override; ObjectEnvironment& object_record() { return *m_object_record; } Object& global_this_value() { return *m_global_this_value; } diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp index facbf7165ae..b5d10d312ea 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp @@ -138,9 +138,11 @@ ThrowCompletionOr ObjectEnvironment::get_binding_value(GlobalObject& glob } // 9.1.1.2.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-object-environment-records-deletebinding-n -bool ObjectEnvironment::delete_binding(GlobalObject&, FlyString const& name) +ThrowCompletionOr ObjectEnvironment::delete_binding(GlobalObject&, FlyString const& name) { - return TRY_OR_DISCARD(m_binding_object.internal_delete(name)); + // 1. Let bindingObject be envRec.[[BindingObject]]. + // 2. Return ? bindingObject.[[Delete]](N). + return m_binding_object.internal_delete(name); } } diff --git a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h index c69f27db96c..6d451b5057a 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h +++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h @@ -26,7 +26,7 @@ public: virtual ThrowCompletionOr initialize_binding(GlobalObject&, FlyString const& name, Value) override; virtual ThrowCompletionOr set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override; virtual ThrowCompletionOr get_binding_value(GlobalObject&, FlyString const& name, bool strict) override; - virtual bool delete_binding(GlobalObject&, FlyString const& name) override; + virtual ThrowCompletionOr delete_binding(GlobalObject&, FlyString const& name) override; // 9.1.1.2.10 WithBaseObject ( ), https://tc39.es/ecma262/#sec-object-environment-records-withbaseobject virtual Object* with_base_object() const override diff --git a/Userland/Libraries/LibJS/Runtime/Reference.cpp b/Userland/Libraries/LibJS/Runtime/Reference.cpp index 3c7a92036fc..10d07609bc8 100644 --- a/Userland/Libraries/LibJS/Runtime/Reference.cpp +++ b/Userland/Libraries/LibJS/Runtime/Reference.cpp @@ -144,7 +144,7 @@ bool Reference::delete_(GlobalObject& global_object) VERIFY(m_base_type == BaseType::Environment); // c. Return ? base.DeleteBinding(ref.[[ReferencedName]]). - return m_base_environment->delete_binding(global_object, m_name.as_string()); + return TRY_OR_DISCARD(m_base_environment->delete_binding(global_object, m_name.as_string())); } String Reference::to_string() const