From 860a37640b24e425da34c086d57fa938595d97f7 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Wed, 20 Oct 2021 08:24:54 -0400 Subject: [PATCH] LibJS: Convert GetIterator AO to ThrowCompletionOr --- Userland/Libraries/LibJS/Bytecode/Op.cpp | 5 ++- .../LibJS/Runtime/ArrayConstructor.cpp | 4 +- .../LibJS/Runtime/Intl/ListFormat.cpp | 4 +- .../LibJS/Runtime/IteratorOperations.cpp | 28 ++++++------ .../LibJS/Runtime/IteratorOperations.h | 3 +- .../LibJS/Runtime/PromiseConstructor.cpp | 45 +++++++++++++------ .../Runtime/Temporal/AbstractOperations.cpp | 4 +- .../Runtime/Temporal/CalendarPrototype.cpp | 4 +- Userland/Libraries/LibJS/Runtime/VM.cpp | 7 +-- 9 files changed, 56 insertions(+), 48 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 3fb6dc9daa..58a0a96b03 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -478,7 +478,10 @@ void PutByValue::execute_impl(Bytecode::Interpreter& interpreter) const void GetIterator::execute_impl(Bytecode::Interpreter& interpreter) const { - interpreter.accumulator() = get_iterator(interpreter.global_object(), interpreter.accumulator()); + auto iterator_or_error = get_iterator(interpreter.global_object(), interpreter.accumulator()); + if (iterator_or_error.is_error()) + return; + interpreter.accumulator() = iterator_or_error.release_value(); } void IteratorNext::execute_impl(Bytecode::Interpreter& interpreter) const diff --git a/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp index 933b40001e..8cc1b3e7a8 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp @@ -116,10 +116,8 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(ArrayConstructor::from) } else { array = Array::create(global_object, 0); } - auto iterator = get_iterator(global_object, items, IteratorHint::Sync, using_iterator); - if (vm.exception()) - return {}; + auto iterator = TRY_OR_DISCARD(get_iterator(global_object, items, IteratorHint::Sync, using_iterator)); auto& array_object = array.as_object(); size_t k = 0; diff --git a/Userland/Libraries/LibJS/Runtime/Intl/ListFormat.cpp b/Userland/Libraries/LibJS/Runtime/Intl/ListFormat.cpp index 4995e51adc..d695056bb4 100644 --- a/Userland/Libraries/LibJS/Runtime/Intl/ListFormat.cpp +++ b/Userland/Libraries/LibJS/Runtime/Intl/ListFormat.cpp @@ -276,9 +276,7 @@ ThrowCompletionOr> string_list_from_iterable(GlobalObject& global } // 2. Let iteratorRecord be ? GetIterator(iterable). - auto* iterator_record = get_iterator(global_object, iterable); - if (auto* exception = vm.exception()) - return throw_completion(exception->value()); + auto* iterator_record = TRY(get_iterator(global_object, iterable)); // 3. Let list be a new empty List. Vector list; diff --git a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp index 5b823dcd68..513a0121e0 100644 --- a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp @@ -12,24 +12,23 @@ namespace JS { // 7.4.1 GetIterator ( obj [ , hint [ , method ] ] ), https://tc39.es/ecma262/#sec-getiterator -Object* get_iterator(GlobalObject& global_object, Value value, IteratorHint hint, Value method) +ThrowCompletionOr get_iterator(GlobalObject& global_object, Value value, IteratorHint hint, Value method) { auto& vm = global_object.vm(); if (method.is_empty()) { if (hint == IteratorHint::Async) TODO(); - auto object = TRY_OR_DISCARD(value.to_object(global_object)); - method = TRY_OR_DISCARD(object->get(*vm.well_known_symbol_iterator())); - } - if (!method.is_function()) { - vm.throw_exception(global_object, ErrorType::NotIterable, value.to_string_without_side_effects()); - return nullptr; - } - auto iterator = TRY_OR_DISCARD(vm.call(method.as_function(), value)); - if (!iterator.is_object()) { - vm.throw_exception(global_object, ErrorType::NotIterable, value.to_string_without_side_effects()); - return nullptr; + auto object = TRY(value.to_object(global_object)); + method = TRY(object->get(*vm.well_known_symbol_iterator())); } + + if (!method.is_function()) + return vm.throw_completion(global_object, ErrorType::NotIterable, value.to_string_without_side_effects()); + + auto iterator = TRY(vm.call(method.as_function(), value)); + if (!iterator.is_object()) + return vm.throw_completion(global_object, ErrorType::NotIterable, value.to_string_without_side_effects()); + return &iterator.as_object(); } @@ -170,9 +169,10 @@ void get_iterator_values(GlobalObject& global_object, Value value, Function +#include #include namespace JS { @@ -18,7 +19,7 @@ enum class IteratorHint { Async, }; -Object* get_iterator(GlobalObject&, Value value, IteratorHint hint = IteratorHint::Sync, Value method = {}); +ThrowCompletionOr get_iterator(GlobalObject&, Value value, IteratorHint hint = IteratorHint::Sync, Value method = {}); Object* iterator_next(Object& iterator, Value value = {}); Object* iterator_step(GlobalObject&, Object& iterator); bool iterator_complete(GlobalObject&, Object& iterator_result); diff --git a/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp b/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp index f576de8f4f..e8b8da0669 100644 --- a/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp @@ -37,16 +37,29 @@ static Value get_promise_resolve(GlobalObject& global_object, Value constructor) } // 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise -static Optional if_abrupt_reject_promise(GlobalObject& global_object, Value, PromiseCapability capability) +template +static Optional if_abrupt_reject_promise(GlobalObject& global_object, ValueType const& value_or_error, PromiseCapability capability) { auto& vm = global_object.vm(); - if (auto* exception = vm.exception()) { - vm.clear_exception(); - vm.stop_unwind(); + // FIXME: This is temporary multi-type support. When all callers to this method are transformed + // to use ThrowCompletionOr, the 'else' clause (and template) can be dropped. + if constexpr (requires { value_or_error.is_throw_completion(); }) { + if (value_or_error.is_throw_completion()) { + vm.clear_exception(); + vm.stop_unwind(); - (void)vm.call(*capability.reject, js_undefined(), exception->value()); - return capability.promise; + (void)vm.call(*capability.reject, js_undefined(), value_or_error.throw_completion().value()); + return capability.promise; + } + } else { + if (auto* exception = vm.exception()) { + vm.clear_exception(); + vm.stop_unwind(); + + (void)vm.call(*capability.reject, js_undefined(), exception->value()); + return capability.promise; + } } return {}; @@ -281,9 +294,10 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(PromiseConstructor::all) if (auto abrupt = if_abrupt_reject_promise(global_object, promise_resolve, promise_capability); abrupt.has_value()) return abrupt.value(); - auto iterator_record = get_iterator(global_object, vm.argument(0)); - if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record, promise_capability); abrupt.has_value()) + auto iterator_record_or_error = get_iterator(global_object, vm.argument(0)); + if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record_or_error, promise_capability); abrupt.has_value()) return abrupt.value(); + auto* iterator_record = iterator_record_or_error.release_value(); auto result = perform_promise_all(global_object, *iterator_record, constructor, promise_capability, promise_resolve); if (vm.exception()) { @@ -310,9 +324,10 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(PromiseConstructor::all_settled) if (auto abrupt = if_abrupt_reject_promise(global_object, promise_resolve, promise_capability); abrupt.has_value()) return abrupt.value(); - auto iterator_record = get_iterator(global_object, vm.argument(0)); - if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record, promise_capability); abrupt.has_value()) + auto iterator_record_or_error = get_iterator(global_object, vm.argument(0)); + if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record_or_error, promise_capability); abrupt.has_value()) return abrupt.value(); + auto* iterator_record = iterator_record_or_error.release_value(); auto result = perform_promise_all_settled(global_object, *iterator_record, constructor, promise_capability, promise_resolve); if (vm.exception()) { @@ -339,9 +354,10 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(PromiseConstructor::any) if (auto abrupt = if_abrupt_reject_promise(global_object, promise_resolve, promise_capability); abrupt.has_value()) return abrupt.value(); - auto iterator_record = get_iterator(global_object, vm.argument(0)); - if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record, promise_capability); abrupt.has_value()) + auto iterator_record_or_error = get_iterator(global_object, vm.argument(0)); + if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record_or_error, promise_capability); abrupt.has_value()) return abrupt.value(); + auto* iterator_record = iterator_record_or_error.release_value(); auto result = perform_promise_any(global_object, *iterator_record, constructor, promise_capability, promise_resolve); if (vm.exception()) { @@ -368,9 +384,10 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(PromiseConstructor::race) if (auto abrupt = if_abrupt_reject_promise(global_object, promise_resolve, promise_capability); abrupt.has_value()) return abrupt.value(); - auto iterator_record = get_iterator(global_object, vm.argument(0)); - if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record, promise_capability); abrupt.has_value()) + auto iterator_record_or_error = get_iterator(global_object, vm.argument(0)); + if (auto abrupt = if_abrupt_reject_promise(global_object, iterator_record_or_error, promise_capability); abrupt.has_value()) return abrupt.value(); + auto* iterator_record = iterator_record_or_error.release_value(); auto result = perform_promise_race(global_object, *iterator_record, constructor, promise_capability, promise_resolve); if (vm.exception()) { diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp index 157315bf27..66eca3d7cf 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp @@ -41,9 +41,7 @@ ThrowCompletionOr iterable_to_list_of_type(GlobalObject& global auto& heap = global_object.heap(); // 1. Let iteratorRecord be ? GetIterator(items, sync). - auto iterator_record = get_iterator(global_object, items, IteratorHint::Sync); - if (auto* exception = vm.exception()) - return throw_completion(exception->value()); + auto iterator_record = TRY(get_iterator(global_object, items, IteratorHint::Sync)); // 2. Let values be a new empty List. MarkedValueList values(heap); diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp index 96333060c0..f6382492e8 100644 --- a/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp @@ -492,9 +492,7 @@ JS_DEFINE_OLD_NATIVE_FUNCTION(CalendarPrototype::fields) VERIFY(calendar->identifier() == "iso8601"sv); // 4. Let iteratorRecord be ? Getiterator(fields, sync). - auto* iterator_record = get_iterator(global_object, fields, IteratorHint::Sync); - if (vm.exception()) - return {}; + auto* iterator_record = TRY_OR_DISCARD(get_iterator(global_object, fields, IteratorHint::Sync)); // 5. Let fieldNames be a new empty List. auto field_names = MarkedValueList { vm.heap() }; diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index ae8965ffd2..22b9e5cef9 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -196,12 +196,7 @@ ThrowCompletionOr VM::binding_initialization(NonnullRefPtr TRY(property_binding_initialization(*target, value, environment, global_object)); return {}; } else { - auto* iterator = get_iterator(global_object, value); - if (!iterator) { - VERIFY(exception()); - return JS::throw_completion(exception()->value()); - } - + auto* iterator = TRY(get_iterator(global_object, value)); auto iterator_done = false; auto result = iterator_binding_initialization(*target, iterator, iterator_done, environment, global_object);