LibJS: Return Optional<T> from Completion::{value,target}(), not T

In the end this is a nicer API than having separate has_{value,target}()
and having to check those first, and then making another Optional from
the unwrapped value:

    completion.has_value() ? completion.value() : Optional<Value> {}
    //                       ^^^^^^^^^^^^^^^^^^
    //         Implicit creation of non-empty Optional<Value>

This way we need to unwrap the optional ourselves, but can easily pass
it to something else as well.

This is in anticipation of the AST using completions :^)

10 files changed, 41 insertions, 43 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/ArrayBufferConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ArrayBufferConstructor.cpp
index c82a467c32..f6781a9750 100644
--- a/Userland/Libraries/LibJS/Runtime/ArrayBufferConstructor.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ArrayBufferConstructor.cpp
@@ -53,7 +53,7 @@ ThrowCompletionOr<Object*> ArrayBufferConstructor::construct(FunctionObject& new
     auto byte_length_or_error = vm.argument(0).to_index(global_object());
     if (byte_length_or_error.is_error()) {
         auto error = byte_length_or_error.release_error();
-        if (error.value().is_object() && is<RangeError>(error.value().as_object())) {
+        if (error.value()->is_object() && is<RangeError>(error.value()->as_object())) {
             // Re-throw more specific RangeError
             vm.clear_exception();
             return vm.throw_completion<RangeError>(global_object(), ErrorType::InvalidLength, "array buffer");
diff --git a/Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp b/Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
index d2c4f84145..08fd1c4d0c 100644
--- a/Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
+++ b/Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp
@@ -41,7 +41,7 @@ ThrowCompletionOr<Value> AsyncFunctionDriverWrapper::react_to_async_task_complet
         vm.clear_exception();
         vm.stop_unwind();
         auto promise = Promise::create(global_object);
-        promise->reject(generator_result.throw_completion().value());
+        promise->reject(*generator_result.throw_completion().value());
         return promise;
     }
 
diff --git a/Userland/Libraries/LibJS/Runtime/Completion.h b/Userland/Libraries/LibJS/Runtime/Completion.h
index ff3f500d78..6e55f69e8e 100644
--- a/Userland/Libraries/LibJS/Runtime/Completion.h
+++ b/Userland/Libraries/LibJS/Runtime/Completion.h
@@ -60,12 +60,10 @@ public:
     }
 
     [[nodiscard]] Type type() const { return m_type; }
-
-    [[nodiscard]] bool has_value() const { return m_value.has_value(); }
-    [[nodiscard]] Value value() const { return *m_value; }
-
-    [[nodiscard]] bool has_target() const { return m_target.has_value(); }
-    [[nodiscard]] FlyString const& target() const { return *m_target; }
+    [[nodiscard]] Optional<Value>& value() { return m_value; }
+    [[nodiscard]] Optional<Value> const& value() const { return m_value; }
+    [[nodiscard]] Optional<FlyString>& target() { return m_target; }
+    [[nodiscard]] Optional<FlyString> const& target() const { return m_target; }
 
     // "abrupt completion refers to any completion with a [[Type]] value other than normal"
     [[nodiscard]] bool is_abrupt() const { return m_type != Type::Normal; }
diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
index 5f44bf8be3..565a5dee4d 100644
--- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
@@ -246,23 +246,23 @@ ThrowCompletionOr<Object*> ECMAScriptFunctionObject::internal_construct(MarkedVa
     if (result.type() == Completion::Type::Return) {
         // FIXME: This is leftover from untangling the call/construct mess - doesn't belong here in any way, but removing it breaks derived classes.
         // Likely fixed by making ClassDefinitionEvaluation fully spec compliant.
-        if (kind == ConstructorKind::Derived && result.value().is_object()) {
+        if (kind == ConstructorKind::Derived && result.value()->is_object()) {
             auto prototype = TRY(new_target.get(vm.names.prototype));
             if (prototype.is_object())
-                TRY(result.value().as_object().internal_set_prototype_of(&prototype.as_object()));
+                TRY(result.value()->as_object().internal_set_prototype_of(&prototype.as_object()));
         }
         // EOF (End of FIXME)
 
         // a. If Type(result.[[Value]]) is Object, return NormalCompletion(result.[[Value]]).
-        if (result.value().is_object())
-            return &result.value().as_object();
+        if (result.value()->is_object())
+            return &result.value()->as_object();
 
         // b. If kind is base, return NormalCompletion(thisArgument).
         if (kind == ConstructorKind::Base)
             return this_argument;
 
         // c. If result.[[Value]] is not undefined, throw a TypeError exception.
-        if (!result.value().is_undefined())
+        if (!result.value()->is_undefined())
             return vm.throw_completion<TypeError>(global_object, ErrorType::DerivedConstructorReturningInvalidValue);
     }
     // 11. Else, ReturnIfAbrupt(result).
@@ -788,7 +788,7 @@ Completion ECMAScriptFunctionObject::ordinary_call_evaluate_body()
 
         VERIFY(result_and_frame.frame != nullptr);
         if (result_and_frame.value.is_error()) {
-            vm.throw_exception(bytecode_interpreter->global_object(), result_and_frame.value.release_error().value());
+            vm.throw_exception(bytecode_interpreter->global_object(), *result_and_frame.value.release_error().value());
             return throw_completion(vm.exception()->value());
         }
         auto result = result_and_frame.value.release_value();
@@ -844,7 +844,7 @@ Completion ECMAScriptFunctionObject::ordinary_call_evaluate_body()
             // 4. Else,
             else {
                 // a. Perform ! Call(promiseCapability.[[Reject]], undefined, « declResult.[[Value]] »).
-                MUST(call(global_object(), promise_capability.reject, js_undefined(), declaration_result.throw_completion().value()));
+                MUST(call(global_object(), promise_capability.reject, js_undefined(), *declaration_result.throw_completion().value()));
             }
 
             // 5. Return Completion { [[Type]]: return, [[Value]]: promiseCapability.[[Promise]], [[Target]]: empty }.
diff --git a/Userland/Libraries/LibJS/Runtime/Promise.cpp b/Userland/Libraries/LibJS/Runtime/Promise.cpp
index 0e55866e5a..9219d3ff53 100644
--- a/Userland/Libraries/LibJS/Runtime/Promise.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Promise.cpp
@@ -116,7 +116,7 @@ Promise::ResolvingFunctions Promise::create_resolving_functions()
             dbgln_if(PROMISE_DEBUG, "[Promise @ {} / PromiseResolvingFunction]: Exception while getting 'then' property, rejecting with error", &promise);
             vm.clear_exception();
             vm.stop_unwind();
-            return promise.reject(then.throw_completion().value());
+            return promise.reject(*then.throw_completion().value());
         }
 
         // 11. Let thenAction be then.[[Value]].
diff --git a/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp b/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp
index bad5a263d2..354aed7d68 100644
--- a/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp
+++ b/Userland/Libraries/LibJS/Runtime/PromiseJobs.cpp
@@ -89,14 +89,14 @@ ThrowCompletionOr<Value> PromiseReactionJob::call()
         // i. Let status be Call(promiseCapability.[[Reject]], undefined, « handlerResult.[[Value]] »).
         auto* reject_function = promise_capability.value().reject;
         dbgln_if(PROMISE_DEBUG, "[PromiseReactionJob @ {}]: Calling PromiseCapability's reject function @ {}", this, reject_function);
-        return vm.call(*reject_function, js_undefined(), handler_result.value());
+        return vm.call(*reject_function, js_undefined(), *handler_result.value());
     }
     // i. Else,
     else {
         // i. Let status be Call(promiseCapability.[[Resolve]], undefined, « handlerResult.[[Value]] »).
         auto* resolve_function = promise_capability.value().resolve;
         dbgln_if(PROMISE_DEBUG, "[PromiseReactionJob @ {}]: Calling PromiseCapability's resolve function @ {}", this, resolve_function);
-        return vm.call(*resolve_function, js_undefined(), handler_result.value());
+        return vm.call(*resolve_function, js_undefined(), *handler_result.value());
     }
 
     // j. Return Completion(status).
@@ -142,8 +142,8 @@ ThrowCompletionOr<Value> PromiseResolveThenableJob::call()
         vm.stop_unwind();
 
         // i. Let status be Call(resolvingFunctions.[[Reject]], undefined, « thenCallResult.[[Value]] »).
-        dbgln_if(PROMISE_DEBUG, "[PromiseResolveThenableJob @ {}]: then_call_result is an abrupt completion, calling reject function with value {}", this, then_call_result.throw_completion().value());
-        auto status = JS::call(global_object, &reject_function, js_undefined(), then_call_result.throw_completion().value());
+        dbgln_if(PROMISE_DEBUG, "[PromiseResolveThenableJob @ {}]: then_call_result is an abrupt completion, calling reject function with value {}", this, *then_call_result.throw_completion().value());
+        auto status = JS::call(global_object, &reject_function, js_undefined(), *then_call_result.throw_completion().value());
 
         // ii. Return Completion(status).
         return status;
diff --git a/Userland/Libraries/LibJS/Runtime/PromiseReaction.h b/Userland/Libraries/LibJS/Runtime/PromiseReaction.h
index 23823fccb1..a223135972 100644
--- a/Userland/Libraries/LibJS/Runtime/PromiseReaction.h
+++ b/Userland/Libraries/LibJS/Runtime/PromiseReaction.h
@@ -20,23 +20,23 @@ struct PromiseCapability {
 };
 
 // 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise
-#define TRY_OR_REJECT(vm, capability, expression)                                                                      \
-    ({                                                                                                                 \
-        auto _temporary_try_or_reject_result = (expression);                                                           \
-        /* 1. If value is an abrupt completion, then */                                                                \
-        if (_temporary_try_or_reject_result.is_error()) {                                                              \
-            vm.clear_exception();                                                                                      \
-            vm.stop_unwind();                                                                                          \
-                                                                                                                       \
-            /* a. Perform ? Call(capability.[[Reject]], undefined, « value.[[Value]] »). */                          \
-            TRY(vm.call(*capability.reject, js_undefined(), _temporary_try_or_reject_result.release_error().value())); \
-                                                                                                                       \
-            /* b. Return capability.[[Promise]]. */                                                                    \
-            return capability.promise;                                                                                 \
-        }                                                                                                              \
-                                                                                                                       \
-        /* 2. Else if value is a Completion Record, set value to value.[[Value]]. */                                   \
-        _temporary_try_or_reject_result.release_value();                                                               \
+#define TRY_OR_REJECT(vm, capability, expression)                                                                       \
+    ({                                                                                                                  \
+        auto _temporary_try_or_reject_result = (expression);                                                            \
+        /* 1. If value is an abrupt completion, then */                                                                 \
+        if (_temporary_try_or_reject_result.is_error()) {                                                               \
+            vm.clear_exception();                                                                                       \
+            vm.stop_unwind();                                                                                           \
+                                                                                                                        \
+            /* a. Perform ? Call(capability.[[Reject]], undefined, « value.[[Value]] »). */                           \
+            TRY(vm.call(*capability.reject, js_undefined(), *_temporary_try_or_reject_result.release_error().value())); \
+                                                                                                                        \
+            /* b. Return capability.[[Promise]]. */                                                                     \
+            return capability.promise;                                                                                  \
+        }                                                                                                               \
+                                                                                                                        \
+        /* 2. Else if value is a Completion Record, set value to value.[[Value]]. */                                    \
+        _temporary_try_or_reject_result.release_value();                                                                \
     })
 
 // 27.2.1.5 NewPromiseCapability ( C ), https://tc39.es/ecma262/#sec-newpromisecapability
diff --git a/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp b/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
index 9e547c4568..9f8c7b4354 100644
--- a/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp
@@ -127,7 +127,7 @@ ThrowCompletionOr<Value> perform_shadow_realm_eval(GlobalObject& global_object,
     }
 
     // 21. If result.[[Type]] is normal and result.[[Value]] is empty, then
-    if (result.type() == Completion::Type::Normal && !result.has_value()) {
+    if (result.type() == Completion::Type::Normal && !result.value().has_value()) {
         // a. Set result to NormalCompletion(undefined).
         result = normal_completion(js_undefined());
     }
@@ -144,7 +144,7 @@ ThrowCompletionOr<Value> perform_shadow_realm_eval(GlobalObject& global_object,
         return vm.throw_completion<TypeError>(global_object, ErrorType::ShadowRealmEvaluateAbruptCompletion);
 
     // 25. Return ? GetWrappedValue(callerRealm, result.[[Value]]).
-    return get_wrapped_value(global_object, caller_realm, result.value());
+    return get_wrapped_value(global_object, caller_realm, *result.value());
 
     // NOTE: Also see "Editor's Note" in the spec regarding the TypeError above.
 }
diff --git a/Userland/Libraries/LibJS/Runtime/TypedArray.cpp b/Userland/Libraries/LibJS/Runtime/TypedArray.cpp
index 6c30db49fa..05fc7dcfd4 100644
--- a/Userland/Libraries/LibJS/Runtime/TypedArray.cpp
+++ b/Userland/Libraries/LibJS/Runtime/TypedArray.cpp
@@ -469,7 +469,7 @@ void TypedArrayBase::visit_edges(Visitor& visitor)
         auto array_length_or_error = first_argument.to_index(global_object());                                                         \
         if (array_length_or_error.is_error()) {                                                                                        \
             auto error = array_length_or_error.release_error();                                                                        \
-            if (error.value().is_object() && is<RangeError>(error.value().as_object())) {                                              \
+            if (error.value()->is_object() && is<RangeError>(error.value()->as_object())) {                                            \
                 /* Re-throw more specific RangeError */                                                                                \
                 vm.clear_exception();                                                                                                  \
                 return vm.throw_completion<RangeError>(global_object(), ErrorType::InvalidLength, "typed array");                      \
diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp
index 3bf65f2aa2..c0705dc61e 100644
--- a/Userland/Libraries/LibJS/Runtime/VM.cpp
+++ b/Userland/Libraries/LibJS/Runtime/VM.cpp
@@ -361,7 +361,7 @@ ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const
                 auto next_object_or_error = iterator_next(*iterator);
                 if (next_object_or_error.is_throw_completion()) {
                     iterator_done = true;
-                    return JS::throw_completion(next_object_or_error.release_error().value());
+                    return JS::throw_completion(*next_object_or_error.release_error().value());
                 }
                 auto* next_object = next_object_or_error.release_value();
 
@@ -380,7 +380,7 @@ ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const
             auto next_object_or_error = iterator_next(*iterator);
             if (next_object_or_error.is_throw_completion()) {
                 iterator_done = true;
-                return JS::throw_completion(next_object_or_error.release_error().value());
+                return JS::throw_completion(*next_object_or_error.release_error().value());
             }
             auto* next_object = next_object_or_error.release_value();
 
@@ -392,7 +392,7 @@ ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const
                 auto value_or_error = next_object->get(names.value);
                 if (value_or_error.is_throw_completion()) {
                     iterator_done = true;
-                    return JS::throw_completion(value_or_error.release_error().value());
+                    return JS::throw_completion(*value_or_error.release_error().value());
                 }
                 value = value_or_error.release_value();
             }