LibJS: Convert the GetValue AO to ThrowCompletionOr

Author: https://github.com/IdanHo
Commit: https://github.com/SerenityOS/serenity/commit/390a04a9851
Pull-request: https://github.com/SerenityOS/serenity/pull/10765
Reviewed-by: https://github.com/linusg ✅

Tests/LibJS/test-js.cpp

@@ -65,9 +65,7 @@ TESTJS_GLOBAL_FUNCTION(mark_as_garbage, markAsGarbage)
 
     auto reference = vm.resolve_binding(variable_name.string(), outer_environment.value()->lexical_environment);
 
-    auto value = reference.get_value(global_object);
+    auto value = TRY(reference.get_value(global_object));
-    if (auto* exception = vm.exception())
-        return JS::throw_completion(exception->value());
 
     if (!value.is_object())
         return vm.throw_completion<JS::TypeError>(global_object, JS::ErrorType::NotAnObject, String::formatted("Variable with name {}", variable_name.string()));

Userland/Libraries/LibJS/AST.cpp

@@ -220,13 +220,9 @@ Value ExpressionStatement::execute(Interpreter& interpreter, GlobalObject& globa
 
 CallExpression::ThisAndCallee CallExpression::compute_this_and_callee(Interpreter& interpreter, GlobalObject& global_object, Reference const& callee_reference) const
 {
-    auto& vm = interpreter.vm();
-
     if (callee_reference.is_property_reference()) {
         auto this_value = callee_reference.get_this_value();
-        auto callee = callee_reference.get_value(global_object);
+        auto callee = TRY_OR_DISCARD(callee_reference.get_value(global_object));
-        if (vm.exception())
-            return {};
 
         return { this_value, callee };
     }
@@ -236,7 +232,7 @@ CallExpression::ThisAndCallee CallExpression::compute_this_and_callee(Interprete
         js_undefined(),
         callee_reference.is_unresolvable()
             ? m_callee->execute(interpreter, global_object)
-            : callee_reference.get_value(global_object)
+            : TRY_OR_DISCARD(callee_reference.get_value(global_object))
     };
 }
 
@@ -1074,7 +1070,7 @@ Reference MemberExpression::to_reference(Interpreter& interpreter, GlobalObject&
     Value base_value;
 
     if (base_reference.is_valid_reference())
-        base_value = base_reference.get_value(global_object);
+        base_value = TRY_OR_DISCARD(base_reference.get_value(global_object));
     else
         base_value = m_object->execute(interpreter, global_object);
 
@@ -1129,13 +1125,10 @@ Value UnaryExpression::execute(Interpreter& interpreter, GlobalObject& global_ob
         if (interpreter.exception())
             return {};
 
-        if (reference.is_unresolvable()) {
+        if (reference.is_unresolvable())
             lhs_result = js_undefined();
-        } else {
+        else
-            lhs_result = reference.get_value(global_object);
+            lhs_result = TRY_OR_DISCARD(reference.get_value(global_object));
-            if (interpreter.exception())
-                return {};
-        }
         VERIFY(!lhs_result.is_empty());
     } else {
         lhs_result = m_lhs->execute(interpreter, global_object);
@@ -1424,9 +1417,7 @@ ThrowCompletionOr<Value> ClassExpression::class_definition_evaluation(Interprete
             return throw_completion(exception->value());
 
         if (reference.is_valid_reference()) {
-            super_class = reference.get_value(global_object);
+            super_class = TRY(reference.get_value(global_object));
-            if (auto* exception = interpreter.exception())
-                return throw_completion(exception->value());
         } else {
             super_class = m_super_class->execute(interpreter, global_object);
             if (auto* exception = interpreter.exception())
@@ -2089,7 +2080,7 @@ Value Identifier::execute(Interpreter& interpreter, GlobalObject& global_object)
     if (interpreter.exception())
         return {};
 
-    return reference.get_value(global_object);
+    return TRY_OR_DISCARD(reference.get_value(global_object));
 }
 
 void Identifier::dump(int indent) const
@@ -2180,9 +2171,7 @@ Value AssignmentExpression::execute(Interpreter& interpreter, GlobalObject& glob
     if (interpreter.exception())
         return {};
 
-    auto lhs_result = reference.get_value(global_object);
+    auto lhs_result = TRY_OR_DISCARD(reference.get_value(global_object));
-    if (interpreter.exception())
-        return {};
 
     //  AssignmentExpression : LeftHandSideExpression {&&=, ||=, ??=} AssignmentExpression
     if (m_op == AssignmentOp::AndAssignment || m_op == AssignmentOp::OrAssignment || m_op == AssignmentOp::NullishAssignment) {
@@ -2289,9 +2278,7 @@ Value UpdateExpression::execute(Interpreter& interpreter, GlobalObject& global_o
 
     if (interpreter.exception())
         return {};
-    auto old_value = reference.get_value(global_object);
+    auto old_value = TRY_OR_DISCARD(reference.get_value(global_object));
-    if (interpreter.exception())
-        return {};
     old_value = TRY_OR_DISCARD(old_value.to_numeric(global_object));
 
     Value new_value;
@@ -2656,7 +2643,7 @@ Value MemberExpression::execute(Interpreter& interpreter, GlobalObject& global_o
     auto reference = to_reference(interpreter, global_object);
     if (interpreter.exception())
         return {};
-    return reference.get_value(global_object);
+    return TRY_OR_DISCARD(reference.get_value(global_object));
 }
 
 bool MemberExpression::ends_in_private_name() const
@@ -2705,7 +2692,7 @@ Optional<OptionalChain::ReferenceAndValue> OptionalChain::to_reference_and_value
 {
     // Note: This is wrapped in an optional to allow base_reference = ...
     Optional<JS::Reference> base_reference = m_base->to_reference(interpreter, global_object);
-    auto base = base_reference->is_unresolvable() ? m_base->execute(interpreter, global_object) : base_reference->get_value(global_object);
+    auto base = base_reference->is_unresolvable() ? m_base->execute(interpreter, global_object) : TRY_OR_DISCARD(base_reference->get_value(global_object));
     if (interpreter.exception())
         return {};
 
@@ -2743,7 +2730,7 @@ Optional<OptionalChain::ReferenceAndValue> OptionalChain::to_reference_and_value
             base = expression->execute(interpreter, global_object);
         } else {
             base_reference = expression->to_reference(interpreter, global_object);
-            base = base_reference->get_value(global_object);
+            base = TRY_OR_DISCARD(base_reference->get_value(global_object));
         }
         if (interpreter.exception())
             return {};

Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -261,7 +261,10 @@ void GetVariable::execute_impl(Bytecode::Interpreter& interpreter) const
     if (interpreter.vm().exception())
         return;
 
-    interpreter.accumulator() = reference.get_value(interpreter.global_object());
+    auto value_or_error = reference.get_value(interpreter.global_object());
+    if (value_or_error.is_error())
+        return;
+    interpreter.accumulator() = value_or_error.release_value();
 }
 
 void SetVariable::execute_impl(Bytecode::Interpreter& interpreter) const

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -24,7 +24,7 @@ void Reference::put_value(GlobalObject& global_object, Value value)
 
     if (is_unresolvable()) {
         if (m_strict) {
-            throw_reference_error(global_object);
+            (void)throw_reference_error(global_object);
             return;
         }
         MUST(global_object.set(m_name, value, Object::ShouldThrowExceptions::No));
@@ -62,38 +62,36 @@ void Reference::put_value(GlobalObject& global_object, Value value)
         (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
+Completion Reference::throw_reference_error(GlobalObject& global_object) const
 {
     auto& vm = global_object.vm();
     if (!m_name.is_valid())
-        vm.throw_exception<ReferenceError>(global_object, ErrorType::ReferenceUnresolvable);
+        return vm.throw_completion<ReferenceError>(global_object, ErrorType::ReferenceUnresolvable);
     else
-        vm.throw_exception<ReferenceError>(global_object, ErrorType::UnknownIdentifier, m_name.to_string_or_symbol().to_display_string());
+        return vm.throw_completion<ReferenceError>(global_object, ErrorType::UnknownIdentifier, m_name.to_string_or_symbol().to_display_string());
 }
 
 // 6.2.4.5 GetValue ( V ), https://tc39.es/ecma262/#sec-getvalue
-Value Reference::get_value(GlobalObject& global_object) const
+ThrowCompletionOr<Value> Reference::get_value(GlobalObject& global_object) const
 {
-    if (!is_valid_reference() || is_unresolvable()) {
+    if (!is_valid_reference() || is_unresolvable())
-        throw_reference_error(global_object);
+        return throw_reference_error(global_object);
-        return {};
-    }
 
     if (is_property_reference()) {
-        auto* base_obj = TRY_OR_DISCARD(m_base_value.to_object(global_object));
+        auto* base_obj = TRY(m_base_value.to_object(global_object));
 
         if (is_private_reference())
-            return TRY_OR_DISCARD(base_obj->private_get(m_private_name));
+            return base_obj->private_get(m_private_name);
 
-        return TRY_OR_DISCARD(base_obj->get(m_name));
+        return base_obj->get(m_name);
     }
 
     VERIFY(m_base_type == BaseType::Environment);
 
     VERIFY(m_base_environment);
     if (m_environment_coordinate.has_value())
-        return TRY_OR_DISCARD(static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(global_object, m_environment_coordinate->index, m_strict));
+        return 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));
+    return 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

Userland/Libraries/LibJS/Runtime/Reference.h

@@ -130,7 +130,7 @@ public:
     }
 
     void put_value(GlobalObject&, Value);
-    Value get_value(GlobalObject&) const;
+    ThrowCompletionOr<Value> get_value(GlobalObject&) const;
     ThrowCompletionOr<bool> delete_(GlobalObject&);
 
     String to_string() const;
@@ -140,7 +140,7 @@ public:
     Optional<EnvironmentCoordinate> environment_coordinate() const { return m_environment_coordinate; }
 
 private:
-    void throw_reference_error(GlobalObject&) const;
+    Completion throw_reference_error(GlobalObject&) const;
 
     BaseType m_base_type { BaseType::Unresolvable };
     union {

Userland/Utilities/js.cpp

@@ -1331,9 +1331,7 @@ int main(int argc, char** argv)
                 auto maybe_variable = vm->resolve_binding(variable_name, &global_environment);
                 if (vm->exception())
                     break;
-                maybe_value = maybe_variable.get_value(interpreter->global_object());
+                maybe_value = TRY_OR_DISCARD(maybe_variable.get_value(interpreter->global_object()));
-                if (vm->exception())
-                    break;
                 VERIFY(!maybe_value->is_empty());
 
                 auto variable = *maybe_value;