LibJS: Convert get_binding_value() to ThrowCompletionOr

Also add spec step comments to it while we're here.
Author: https://github.com/linusg Commit: https://github.com/SerenityOS/serenity/commit/f35e2680241 Pull-request: https://github.com/SerenityOS/serenity/pull/10413 Reviewed-by: https://github.com/IdanHo ✅
2021-10-09 19:43:19 +01:00 · 2021-10-09 19:43:19 +01:00 · f35e268024 · 2024-07-18 02:52:16 +09:00
commit f35e268024
parent 7652138ce0
11 changed files with 43 additions and 27 deletions
--- a/Userland/Libraries/LibJS/AST.cpp
+++ b/Userland/Libraries/LibJS/AST.cpp
@ -168,7 +168,7 @@ Value FunctionDeclaration::execute(Interpreter& interpreter, GlobalObject& globa
        // Perform special annexB steps see step 3 of: https://tc39.es/ecma262/#sec-web-compat-functiondeclarationinstantiation
        auto* variable_environment = interpreter.vm().running_execution_context().variable_environment;
        auto* lexical_environment = interpreter.vm().running_execution_context().lexical_environment;
-        auto function_object = lexical_environment->get_binding_value(global_object, name(), false);
+        auto function_object = MUST(lexical_environment->get_binding_value(global_object, name(), false));
        MUST(variable_environment->set_mutable_binding(global_object, name(), function_object, false));
    }

@ -573,9 +573,7 @@ Value ForStatement::execute(Interpreter& interpreter, GlobalObject& global_objec
        auto* this_iteration_env = new_declarative_environment(*outer);
        for (auto& name : let_declarations) {
            MUST(this_iteration_env->create_mutable_binding(global_object, name, false));
-            auto last_value = last_iteration_env->get_binding_value(global_object, name, true);
-            if (auto* exception = interpreter.exception())
-                return throw_completion(exception->value());
+            auto last_value = TRY(last_iteration_env->get_binding_value(global_object, name, true));
            VERIFY(!last_value.is_empty());
            MUST(this_iteration_env->initialize_binding(global_object, name, last_value));
        }
--- a/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp
@ -829,7 +829,7 @@ Object* create_mapped_arguments_object(GlobalObject& global_object, FunctionObje
            object->parameter_map().define_native_accessor(
                PropertyName { index },
                [&environment, name](VM&, GlobalObject& global_object_getter) -> Value {
-                    return environment.get_binding_value(global_object_getter, name, false);
+                    return MUST(environment.get_binding_value(global_object_getter, name, false));
                },
                [&environment, name](VM& vm, GlobalObject& global_object_setter) {
                    MUST(environment.set_mutable_binding(global_object_setter, name, vm.argument(0), false));
--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.cpp
@ -152,20 +152,25 @@ ThrowCompletionOr<void> DeclarativeEnvironment::set_mutable_binding_direct(Globa
 }

 // 9.1.1.1.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-declarative-environment-records-getbindingvalue-n-s
-Value DeclarativeEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
+ThrowCompletionOr<Value> DeclarativeEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
 {
+    // 1. Assert: envRec has a binding for N.
    auto it = m_names.find(name);
    VERIFY(it != m_names.end());
+
+    // 2-3. (extracted into a non-standard function below)
    return get_binding_value_direct(global_object, it->value, strict);
 }

-Value DeclarativeEnvironment::get_binding_value_direct(GlobalObject& global_object, size_t index, bool)
+ThrowCompletionOr<Value> DeclarativeEnvironment::get_binding_value_direct(GlobalObject& global_object, size_t index, bool)
 {
    auto& binding = m_bindings[index];
-    if (!binding.initialized) {
-        global_object.vm().throw_exception<ReferenceError>(global_object, ErrorType::BindingNotInitialized, name_from_index(index));
-        return {};
-    }
+
+    // 2. If the binding for N in envRec is an uninitialized binding, throw a ReferenceError exception.
+    if (!binding.initialized)
+        return vm().throw_completion<ReferenceError>(global_object, ErrorType::BindingNotInitialized, name_from_index(index));
+
+    // 3. Return the value currently bound to N in envRec.
    return binding.value;
 }

--- a/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/DeclarativeEnvironment.h
@ -27,7 +27,7 @@ public:
    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 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 ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;

    void initialize_or_set_mutable_binding(Badge<ScopeNode>, GlobalObject& global_object, FlyString const& name, Value value);
@ -35,7 +35,7 @@ public:
    // This is not a method defined in the spec! Do not use this in any LibJS (or other spec related) code.
    [[nodiscard]] Vector<String> bindings() const;

-    Value get_binding_value_direct(GlobalObject&, size_t index, bool strict);
+    ThrowCompletionOr<Value> get_binding_value_direct(GlobalObject&, size_t index, bool strict);
    ThrowCompletionOr<void> set_mutable_binding_direct(GlobalObject&, size_t index, Value, bool strict);

 protected:
--- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
@ -464,7 +464,7 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
                if (!parameter_names.contains(name) || function_names.contains(name))
                    initial_value = js_undefined();
                else
-                    initial_value = environment->get_binding_value(global_object(), name, false);
+                    initial_value = MUST(environment->get_binding_value(global_object(), name, false));

                MUST(var_environment->initialize_binding(global_object(), name, initial_value));

--- a/Userland/Libraries/LibJS/Runtime/Environment.h
+++ b/Userland/Libraries/LibJS/Runtime/Environment.h
@ -38,7 +38,7 @@ public:
    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 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 ThrowCompletionOr<Value> 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; }

    // [[OuterEnv]]
--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
@ -107,10 +107,17 @@ ThrowCompletionOr<void> GlobalEnvironment::set_mutable_binding(GlobalObject& glo
 }

 // 9.1.1.4.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-global-environment-records-getbindingvalue-n-s
-Value GlobalEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
+ThrowCompletionOr<Value> GlobalEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
 {
-    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.GetBindingValue(N, S).
        return m_declarative_record->get_binding_value(global_object, name, strict);
+    }
+
+    // 3. Let ObjRec be envRec.[[ObjectRecord]].
+    // 4. Return ? ObjRec.GetBindingValue(N, S).
    return m_object_record->get_binding_value(global_object, name, strict);
 }

--- a/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/GlobalEnvironment.h
@ -24,7 +24,7 @@ public:
    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 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 ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;

    ObjectEnvironment& object_record() { return *m_object_record; }
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.cpp
@ -117,18 +117,24 @@ ThrowCompletionOr<void> ObjectEnvironment::set_mutable_binding(GlobalObject& glo
 }

 // 9.1.1.2.6 GetBindingValue ( N, S ), https://tc39.es/ecma262/#sec-object-environment-records-getbindingvalue-n-s
-Value ObjectEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
+ThrowCompletionOr<Value> ObjectEnvironment::get_binding_value(GlobalObject& global_object, FlyString const& name, bool strict)
 {
    auto& vm = this->vm();
-    auto value = TRY_OR_DISCARD(m_binding_object.has_property(name));
+
+    // 1. Let bindingObject be envRec.[[BindingObject]].
+    // 2. Let value be ? HasProperty(bindingObject, N).
+    auto value = TRY(m_binding_object.has_property(name));
+
+    // 3. If value is false, then
    if (!value) {
+        // a. If S is false, return the value undefined; otherwise throw a ReferenceError exception.
        if (!strict)
            return js_undefined();
-
-        vm.throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
-        return {};
+        return vm.throw_completion<ReferenceError>(global_object, ErrorType::UnknownIdentifier, name);
    }
-    return TRY_OR_DISCARD(m_binding_object.get(name));
+
+    // 4. Return ? Get(bindingObject, N).
+    return m_binding_object.get(name);
 }

 // 9.1.1.2.7 DeleteBinding ( N ), https://tc39.es/ecma262/#sec-object-environment-records-deletebinding-n
--- a/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
+++ b/Userland/Libraries/LibJS/Runtime/ObjectEnvironment.h
@ -25,7 +25,7 @@ public:
    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 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 ThrowCompletionOr<Value> get_binding_value(GlobalObject&, FlyString const& name, bool strict) override;
    virtual bool delete_binding(GlobalObject&, FlyString const& name) override;

    // 9.1.1.2.10 WithBaseObject ( ), https://tc39.es/ecma262/#sec-object-environment-records-withbaseobject
--- a/Userland/Libraries/LibJS/Runtime/Reference.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Reference.cpp
@ -84,8 +84,8 @@ Value Reference::get_value(GlobalObject& global_object) const

    VERIFY(m_base_environment);
    if (m_environment_coordinate.has_value())
-        return static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(global_object, m_environment_coordinate->index, m_strict);
-    return m_base_environment->get_binding_value(global_object, m_name.as_string(), m_strict);
+        return TRY_OR_DISCARD(static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(global_object, m_environment_coordinate->index, m_strict));
+    return TRY_OR_DISCARD(m_base_environment->get_binding_value(global_object, m_name.as_string(), m_strict));
 }

 // 13.5.1.2 Runtime Semantics: Evaluation, https://tc39.es/ecma262/#sec-delete-operator-runtime-semantics-evaluation