LibJS: Convert set_mutable_binding() to ThrowCompletionOr

Also add spec step comments to it while we're here.

Userland/Libraries/LibJS/AST.cpp

@@ -166,9 +166,10 @@ Value FunctionDeclaration::execute(Interpreter& interpreter, GlobalObject& globa
 
     if (m_is_hoisted) {
         // Perform special annexB steps see step 3 of: https://tc39.es/ecma262/#sec-web-compat-functiondeclarationinstantiation
-        auto function_object = interpreter.vm().running_execution_context().lexical_environment->get_binding_value(global_object, name(), false);
+        auto* variable_environment = interpreter.vm().running_execution_context().variable_environment;
-        interpreter.vm().running_execution_context().variable_environment->set_mutable_binding(global_object, name(), function_object, false);
+        auto* lexical_environment = interpreter.vm().running_execution_context().lexical_environment;
-        VERIFY(!interpreter.exception());
+        auto function_object = lexical_environment->get_binding_value(global_object, name(), false);
+        MUST(variable_environment->set_mutable_binding(global_object, name(), function_object, false));
     }
 
     return {};

Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -705,9 +705,7 @@ ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& glo
                 TRY(variable_environment->create_mutable_binding(global_object, declaration.name(), true));
                 TRY(variable_environment->initialize_binding(global_object, declaration.name(), function));
             } else {
-                variable_environment->set_mutable_binding(global_object, declaration.name(), function, false);
+                TRY(variable_environment->set_mutable_binding(global_object, declaration.name(), function, false));
-                if (auto* exception = vm.exception())
-                    return throw_completion(exception->value());
             }
         }
     }
@@ -834,7 +832,7 @@ Object* create_mapped_arguments_object(GlobalObject& global_object, FunctionObje
                     return environment.get_binding_value(global_object_getter, name, false);
                 },
                 [&environment, name](VM& vm, GlobalObject& global_object_setter) {
-                    environment.set_mutable_binding(global_object_setter, name, vm.argument(0), false);
+                    MUST(environment.set_mutable_binding(global_object_setter, name, vm.argument(0), false));
                     return js_undefined();
                 },
                 Attribute::Configurable);

Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp

@@ -106,40 +106,49 @@ ThrowCompletionOr<void> DeclarativeEnvironment::initialize_binding(GlobalObject&
 }
 
 // 9.1.1.1.5 SetMutableBinding ( N, V, S ), https://tc39.es/ecma262/#sec-declarative-environment-records-setmutablebinding-n-v-s
-void DeclarativeEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
+ThrowCompletionOr<void> DeclarativeEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
 {
+    // 1. If envRec does not have a binding for N, then
     auto it = m_names.find(name);
     if (it == m_names.end()) {
-        if (strict) {
+        // a. If S is true, throw a ReferenceError exception.
-            global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
+        if (strict)
-            return;
+            return vm().throw_completion<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
-        }
+
-        (void)create_mutable_binding(global_object, name, true);
+        // b. Perform envRec.CreateMutableBinding(N, true).
-        (void)initialize_binding(global_object, name, value);
+        MUST(create_mutable_binding(global_object, name, true));
-        return;
+
+        // c. Perform envRec.InitializeBinding(N, V).
+        MUST(initialize_binding(global_object, name, value));
+
+        // d. Return NormalCompletion(empty).
+        return {};
     }
 
-    set_mutable_binding_direct(global_object, it->value, value, strict);
+    // 2-5. (extracted into a non-standard function below)
+    TRY(set_mutable_binding_direct(global_object, it->value, value, strict));
+
+    // 6. Return NormalCompletion(empty).
+    return {};
 }
 
-void DeclarativeEnvironment::set_mutable_binding_direct(GlobalObject& global_object, size_t index, Value value, bool strict)
+ThrowCompletionOr<void> DeclarativeEnvironment::set_mutable_binding_direct(GlobalObject& global_object, size_t index, Value value, bool strict)
 {
     auto& binding = m_bindings[index];
     if (binding.strict)
         strict = true;
 
-    if (!binding.initialized) {
+    if (!binding.initialized)
-        global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::BindingNotInitialized, name_from_index(index));
+        return vm().throw_completion<ReferenceError>(global_object, ErrorType::BindingNotInitialized, name_from_index(index));
-        return;
-    }
 
     if (binding.mutable_) {
         binding.value = value;
     } else {
-        if (strict) {
+        if (strict)
-            global_object.vm().throw_exception<TypeError>(global_object, ErrorType::InvalidAssignToConst);
+            return vm().throw_completion<TypeError>(global_object, ErrorType::InvalidAssignToConst);
-        }
     }
+
+    return {};
 }
 
 // 9.1.1.1.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-declarative-environment-records-getbindingvalue-n-s
@@ -182,7 +191,7 @@ void DeclarativeEnvironment::initialize_or_set_mutable_binding(Badge<ScopeNode>,
     if (!binding.initialized)
         MUST(initialize_binding(global_object, name, value));
     else
-        set_mutable_binding(global_object, name, value, false);
+        MUST(set_mutable_binding(global_object, name, value, false));
 }
 
 Vector<String> DeclarativeEnvironment::bindings() const

Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h

@@ -26,7 +26,7 @@ public:
     virtual ThrowCompletionOr<void> create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual ThrowCompletionOr<void> create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
-    virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
+    virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
     virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
 
@@ -36,7 +36,7 @@ public:
     [[nodiscard]] Vector<String> bindings() const;
 
     Value get_binding_value_direct(GlobalObject&, size_t index, bool strict);
-    void set_mutable_binding_direct(GlobalObject&, size_t index, Value, bool strict);
+    ThrowCompletionOr<void> set_mutable_binding_direct(GlobalObject&, size_t index, Value, bool strict);
 
 protected:
     virtual void visit_edges(Visitor&) override;

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -534,7 +534,7 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
 
     for (auto& declaration : functions_to_initialize) {
         auto* function = ECMAScriptFunctionObject::create(global_object(), declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), lex_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.contains_direct_call_to_eval());
-        var_environment->set_mutable_binding(global_object(), declaration.name(), function, false);
+        MUST(var_environment->set_mutable_binding(global_object(), declaration.name(), function, false));
     }
 
     return {};

Userland/Libraries/LibJS/Runtime/Environment.h

@@ -37,7 +37,7 @@ public:
     virtual ThrowCompletionOr<void> create_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool can_be_deleted) { return {}; }
     virtual ThrowCompletionOr<void> create_immutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return {}; }
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value) { return {}; }
-    virtual void set_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value, [[maybe_unused]] bool strict) { }
+    virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, [[maybe_unused]] FlyString const& name, Value, [[maybe_unused]] bool strict) { return {}; }
     virtual Value get_binding_value(GlobalObject&, [[maybe_unused]] FlyString const& name, [[maybe_unused]] bool strict) { return {}; }
     virtual bool delete_binding(GlobalObject&, [[maybe_unused]] FlyString const& name) { return false; }
 

Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp

@@ -92,14 +92,18 @@ ThrowCompletionOr<void> GlobalEnvironment::initialize_binding(GlobalObject& glob
 }
 
 // 9.1.1.4.5 SetMutableBinding ( N, V, S ), https://tc39.es/ecma262/#sec-global-environment-records-setmutablebinding-n-v-s
-void GlobalEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
+ThrowCompletionOr<void> GlobalEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
 {
+    // 1. Let DclRec be envRec.[[DeclarativeRecord]].
+    // 2. If DclRec.HasBinding(N) is true, then
     if (MUST(m_declarative_record->has_binding(name))) {
-        m_declarative_record->set_mutable_binding(global_object, name, value, strict);
+        // a. Return DclRec.SetMutableBinding(N, V, S).
-        return;
+        return m_declarative_record->set_mutable_binding(global_object, name, value, strict);
     }
 
-    m_object_record->set_mutable_binding(global_object, name, value, strict);
+    // 3. Let ObjRec be envRec.[[ObjectRecord]].
+    // 4. Return ? ObjRec.SetMutableBinding(N, V, S).
+    return m_object_record->set_mutable_binding(global_object, name, value, strict);
 }
 
 // 9.1.1.4.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s

Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h

@@ -23,7 +23,7 @@ public:
     virtual ThrowCompletionOr<void> create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual ThrowCompletionOr<void> create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
-    virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
+    virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
     virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
 

Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp

@@ -79,38 +79,41 @@ ThrowCompletionOr<void> ObjectEnvironment::create_immutable_binding(GlobalObject
 ThrowCompletionOr<void> ObjectEnvironment::initialize_binding(GlobalObject& global_object, FlyString const& name, Value value)
 {
     // 1. Return ? envRec.SetMutableBinding(N, V, false).
-    set_mutable_binding(global_object, name, value, false);
+    return set_mutable_binding(global_object, name, value, false);
-    if (auto* exception = vm().exception())
-        return throw_completion(exception->value());
-    return {};
 }
 
 // 9.1.1.2.5 SetMutableBinding ( N, V, S ), https://tc39.es/ecma262/#sec-object-environment-records-setmutablebinding-n-v-s
-void ObjectEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
+ThrowCompletionOr<void> ObjectEnvironment::set_mutable_binding(GlobalObject& global_object, FlyString const& name, Value value, bool strict)
 {
     auto& vm = this->vm();
-    auto still_exists_or_error = m_binding_object.has_property(name);
-    if (still_exists_or_error.is_error())
-        return;
-    auto still_exists = still_exists_or_error.release_value();
-    if (!still_exists && strict) {
-        global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
-        return;
-    }
 
+    // 1. Let bindingObject be envRec.[[BindingObject]].
+    // 2. Let stillExists be ? HasProperty(bindingObject, N).
+    auto still_exists = TRY(m_binding_object.has_property(name));
+
+    // 3. If stillExists is false and S is true, throw a ReferenceError exception.
+    if (!still_exists && strict)
+        return vm.throw_completion<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
+
+    // 4. Return ? Set(bindingObject, N, V, S).
     auto result_or_error = m_binding_object.set(name, value, strict ? Object::ShouldThrowExceptions::Yes : Object::ShouldThrowExceptions::No);
 
     // Note: Nothing like this in the spec, this is here to produce nicer errors instead of the generic one thrown by Object::set().
     if (result_or_error.is_error() && strict) {
         auto property_or_error = m_binding_object.internal_get_own_property(name);
+        // Return the initial error instead of masking it with the new error
         if (property_or_error.is_error())
-            return;
+            return result_or_error.release_error();
         auto property = property_or_error.release_value();
         if (property.has_value() && !property->writable.value_or(true)) {
             vm.clear_exception();
-            vm.throw_exception<TypeError>(global_object, ErrorType::DescWriteNonWritable, name);
+            return vm.throw_completion<TypeError>(global_object, ErrorType::DescWriteNonWritable, name);
         }
     }
+
+    if (result_or_error.is_error())
+        return result_or_error.release_error();
+    return {};
 }
 
 // 9.1.1.2.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-object-environment-records-getbindingvalue-n-s

Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h

@@ -24,7 +24,7 @@ public:
     virtual ThrowCompletionOr<void> create_mutable_binding(GlobalObject&, FlyString const& name, bool can_be_deleted) override;
     virtual ThrowCompletionOr<void> create_immutable_binding(GlobalObject&, FlyString const& name, bool strict) override;
     virtual ThrowCompletionOr<void> initialize_binding(GlobalObject&, FlyString const& name, Value) override;
-    virtual void set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
+    virtual ThrowCompletionOr<void> set_mutable_binding(GlobalObject&, FlyString const& name, Value, bool strict) override;
     virtual Value get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
     virtual bool delete_binding(GlobalObject&, FlyString const& name) override;
 

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -49,10 +49,11 @@ void Reference::put_value(GlobalObject& global_object, Value value)
 
     VERIFY(m_base_type == BaseType::Environment);
     VERIFY(m_base_environment);
+    // These can throw, TRY() when converting put_value() to ThrowCompletionOr
     if (m_environment_coordinate.has_value())
-        static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(global_object, m_environment_coordinate->index, value, m_strict);
+        (void)static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(global_object, m_environment_coordinate->index, value, m_strict);
     else
-        m_base_environment->set_mutable_binding(global_object, m_name.as_string(), value, m_strict);
+        (void)m_base_environment->set_mutable_binding(global_object, m_name.as_string(), value, m_strict);
 }
 
 void Reference::throw_reference_error(GlobalObject& global_object) const