Browse Source

LibJS: Use a Variant instead of two Optionals for ThrowCompletionOr

Comes with the usual benefit of saving some space on the stack, as well
as making a situation where both or neither Optionals hold a value
impossible.

The various unwrapping additions are required as we can no longer
construct a ThrowCompletionOr<T> from an Optional<T> - rightfully so.
Linus Groh 2 years ago
parent
commit
8f1d13e73b

+ 4 - 4
Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2020-2022, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2020-2023, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -184,7 +184,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
                 auto error = vm.throw_completion<TypeError>(ErrorType::ArrayMaxSize);
 
                 // 2. Return ? IteratorClose(iteratorRecord, error).
-                return TRY(iterator_close(vm, iterator, move(error)));
+                return *TRY(iterator_close(vm, iterator, move(error)));
             }
 
             // ii. Let Pk be ! ToString(𝔽(k)).
@@ -214,7 +214,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
 
                 // 2. IfAbruptCloseIterator(mappedValue, iteratorRecord).
                 if (mapped_value_or_error.is_error())
-                    return TRY(iterator_close(vm, iterator, mapped_value_or_error.release_error()));
+                    return *TRY(iterator_close(vm, iterator, mapped_value_or_error.release_error()));
                 mapped_value = mapped_value_or_error.release_value();
             }
             // vii. Else, let mappedValue be nextValue.
@@ -227,7 +227,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
 
             // IfAbruptCloseIterator(defineStatus, iteratorRecord).
             if (result_or_error.is_error())
-                return TRY(iterator_close(vm, iterator, result_or_error.release_error()));
+                return *TRY(iterator_close(vm, iterator, result_or_error.release_error()));
 
             // x. Set k to k + 1.
         }

+ 15 - 16
Userland/Libraries/LibJS/Runtime/Completion.h

@@ -274,23 +274,23 @@ class [[nodiscard]] ThrowCompletionOr {
 public:
     ThrowCompletionOr()
     requires(IsSame<ValueType, Empty>)
-        : m_value(Empty {})
+        : m_value_or_throw_completion(Empty {})
     {
     }
 
     // Not `explicit` on purpose so that `return vm.throw_completion<Error>(...);` is possible.
     ThrowCompletionOr(Completion throw_completion)
-        : m_throw_completion(move(throw_completion))
+        : m_value_or_throw_completion(move(throw_completion))
     {
-        VERIFY(m_throw_completion->is_error());
+        VERIFY(m_value_or_throw_completion.template get<Completion>().is_error());
     }
 
     // Not `explicit` on purpose so that `return value;` is possible.
     ThrowCompletionOr(ValueType value)
-        : m_value(move(value))
+        : m_value_or_throw_completion(move(value))
     {
         if constexpr (IsSame<ValueType, Value>)
-            VERIFY(!m_value->is_empty());
+            VERIFY(!m_value_or_throw_completion.template get<ValueType>().is_empty());
     }
 
     ThrowCompletionOr(ThrowCompletionOr const&) = default;
@@ -299,7 +299,7 @@ public:
     ThrowCompletionOr& operator=(ThrowCompletionOr&&) = default;
 
     ThrowCompletionOr(OptionalNone value)
-        : m_value(ValueType { value })
+        : m_value_or_throw_completion(ValueType { value })
     {
     }
 
@@ -309,28 +309,28 @@ public:
     template<typename WrappedValueType>
     ThrowCompletionOr(WrappedValueType const& value)
     requires(!IsPOD<ValueType>)
-        : m_value(value)
+        : m_value_or_throw_completion(ValueType { value })
     {
     }
 
-    [[nodiscard]] bool is_throw_completion() const { return m_throw_completion.has_value(); }
-    Completion const& throw_completion() const { return *m_throw_completion; }
+    [[nodiscard]] bool is_throw_completion() const { return m_value_or_throw_completion.template has<Completion>(); }
+    Completion const& throw_completion() const { return m_value_or_throw_completion.template get<Completion>(); }
 
     [[nodiscard]] bool has_value() const
     requires(!IsSame<ValueType, Empty>)
     {
-        return m_value.has_value();
+        return m_value_or_throw_completion.template has<ValueType>();
     }
     [[nodiscard]] ValueType const& value() const
     requires(!IsSame<ValueType, Empty>)
     {
-        return *m_value;
+        return m_value_or_throw_completion.template get<ValueType>();
     }
 
     // These are for compatibility with the TRY() macro in AK.
-    [[nodiscard]] bool is_error() const { return m_throw_completion.has_value(); }
-    [[nodiscard]] ValueType release_value() { return m_value.release_value(); }
-    Completion release_error() { return m_throw_completion.release_value(); }
+    [[nodiscard]] bool is_error() const { return m_value_or_throw_completion.template has<Completion>(); }
+    [[nodiscard]] ValueType release_value() { return move(m_value_or_throw_completion.template get<ValueType>()); }
+    Completion release_error() { return move(m_value_or_throw_completion.template get<Completion>()); }
 
     ValueType release_allocated_value_but_fixme_should_propagate_errors()
     {
@@ -339,8 +339,7 @@ public:
     }
 
 private:
-    Optional<Completion> m_throw_completion;
-    Optional<ValueType> m_value;
+    Variant<ValueType, Completion> m_value_or_throw_completion;
 };
 
 template<>

+ 1 - 1
Userland/Libraries/LibJS/Runtime/DisposableStackPrototype.cpp

@@ -69,7 +69,7 @@ JS_DEFINE_NATIVE_FUNCTION(DisposableStackPrototype::dispose)
     disposable_stack->set_disposed();
 
     // 5. Return DisposeResources(disposableStack, NormalCompletion(undefined)).
-    return TRY(dispose_resources(vm, disposable_stack->disposable_resource_stack(), Completion { js_undefined() }));
+    return *TRY(dispose_resources(vm, disposable_stack->disposable_resource_stack(), Completion { js_undefined() }));
 }
 
 // 11.3.3.3 DisposableStack.prototype.use( value ), https://tc39.es/proposal-explicit-resource-management/#sec-disposablestack.prototype.use

+ 2 - 2
Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2020, Stephan Unverwerth <s.unverwerth@serenityos.org>
- * Copyright (c) 2020-2022, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2020-2023, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2023, Andreas Kling <kling@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -183,7 +183,7 @@ ThrowCompletionOr<Value> ECMAScriptFunctionObject::internal_call(Value this_argu
 
     // 8. If result.[[Type]] is return, return result.[[Value]].
     if (result.type() == Completion::Type::Return)
-        return result.value();
+        return *result.value();
 
     // 9. ReturnIfAbrupt(result).
     if (result.is_abrupt()) {

+ 3 - 3
Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp

@@ -558,7 +558,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<TypeError>(ErrorType::TemporalInvalidCalendarFieldValue, TRY_OR_THROW_OOM(vm, next_value.to_string_without_side_effects()));
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(vm, iterator_record, move(completion)));
+            return *TRY(iterator_close(vm, iterator_record, move(completion)));
         }
 
         auto next_value_string = TRY(next_value.as_string().utf8_string());
@@ -569,7 +569,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<RangeError>(ErrorType::TemporalDuplicateCalendarField, next_value_string);
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(vm, iterator_record, move(completion)));
+            return *TRY(iterator_close(vm, iterator_record, move(completion)));
         }
 
         // iv. If nextValue is not one of "year", "month", "monthCode", "day", "hour", "minute", "second", "millisecond", "microsecond", "nanosecond", then
@@ -578,7 +578,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<RangeError>(ErrorType::TemporalInvalidCalendarFieldName, next_value_string);
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(vm, iterator_record, move(completion)));
+            return *TRY(iterator_close(vm, iterator_record, move(completion)));
         }
 
         // v. Append nextValue to the end of the List fieldNames.

+ 1 - 1
Userland/Libraries/LibWeb/HTML/CrossOrigin/AbstractOperations.cpp

@@ -186,7 +186,7 @@ JS::ThrowCompletionOr<JS::Value> cross_origin_get(JS::VM& vm, JS::Object const&
 
     // 3. If IsDataDescriptor(desc) is true, then return desc.[[Value]].
     if (descriptor->is_data_descriptor())
-        return descriptor->value;
+        return *descriptor->value;
 
     // 4. Assert: IsAccessorDescriptor(desc) is true.
     VERIFY(descriptor->is_accessor_descriptor());