LibJS: Implement proper Iterator records

Instead of using plain objects as Iterator records, causes confusion
about the object itself actually being its [[Iterator]] slot, and
requires non-standard type conversion shenanigans fpr the [[NextValue]]
and [[Done]] internal slots,  implement a proper Iterator record struct
and use it throughout.

Also annotate the remaining Iterator AOs with spec comments while we're
here.

Userland/Libraries/LibJS/AST.cpp

@@ -1135,8 +1135,7 @@ Completion ForAwaitOfStatement::loop_evaluation(Interpreter& interpreter, Global
 
     // NOTE: Perform step 7 from ForIn/OfHeadEvaluation. And since this is always async we only have to do step 7.d.
     // d. Return ? GetIterator(exprValue, iteratorHint).
-    auto* iterator = TRY(get_iterator(global_object, rhs_result, IteratorHint::Async));
+    auto iterator = TRY(get_iterator(global_object, rhs_result, IteratorHint::Async));
-    VERIFY(iterator);
 
     auto& vm = interpreter.vm();
 
@@ -1155,15 +1154,12 @@ Completion ForAwaitOfStatement::loop_evaluation(Interpreter& interpreter, Global
 
     // 6. Repeat,
     while (true) {
-        // NOTE: Since we don't have iterator records yet we have to extract the function first.
-        auto next_method = TRY(iterator->get(vm.names.next));
-        if (!next_method.is_function())
-            return vm.throw_completion<TypeError>(global_object, ErrorType::IterableNextNotAFunction);
-
         // a. Let nextResult be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]).
-        auto next_result = TRY(call(global_object, next_method, iterator));
+        auto next_result = TRY(call(global_object, iterator.next_method, iterator.iterator));
+
         // b. If iteratorKind is async, set nextResult to ? Await(nextResult).
         next_result = TRY(await(global_object, next_result));
+
         // c. If Type(nextResult) is not Object, throw a TypeError exception.
         if (!next_result.is_object())
             return vm.throw_completion<TypeError>(global_object, ErrorType::IterableNextBadReturn);
@@ -1193,7 +1189,7 @@ Completion ForAwaitOfStatement::loop_evaluation(Interpreter& interpreter, Global
             auto status = result.update_empty(last_value);
 
             // 3. If iteratorKind is async, return ? AsyncIteratorClose(iteratorRecord, status).
-            return async_iterator_close(*iterator, move(status));
+            return async_iterator_close(global_object, iterator, move(status));
         }
 
         // o. If result.[[Value]] is not empty, set V to result.[[Value]].

Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2021, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2021, Gunnar Beutner <gbeutner@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -16,6 +16,7 @@
 #include <LibJS/Runtime/ECMAScriptFunctionObject.h>
 #include <LibJS/Runtime/Environment.h>
 #include <LibJS/Runtime/GlobalObject.h>
+#include <LibJS/Runtime/Iterator.h>
 #include <LibJS/Runtime/IteratorOperations.h>
 #include <LibJS/Runtime/RegExpObject.h>
 #include <LibJS/Runtime/Value.h>
@@ -132,19 +133,41 @@ void NewArray::execute_impl(Bytecode::Interpreter& interpreter) const
     interpreter.accumulator() = Array::create_from(interpreter.global_object(), elements);
 }
 
+// FIXME: Since the accumulator is a Value, we store an object there and have to convert back and forth between that an Iterator records. Not great.
+// Make sure to put this into the accumulator before the iterator object disappears from the stack to prevent the members from being GC'd.
+static Object* iterator_to_object(GlobalObject& global_object, Iterator iterator)
+{
+    auto& vm = global_object.vm();
+    auto* object = Object::create(global_object, nullptr);
+    object->define_direct_property(vm.names.iterator, iterator.iterator, 0);
+    object->define_direct_property(vm.names.next, iterator.next_method, 0);
+    object->define_direct_property(vm.names.done, Value(iterator.done), 0);
+    return object;
+}
+
+static Iterator object_to_iterator(GlobalObject& global_object, Object& object)
+{
+    auto& vm = global_object.vm();
+    return Iterator {
+        .iterator = &MUST(object.get(vm.names.iterator)).as_object(),
+        .next_method = MUST(object.get(vm.names.next)),
+        .done = MUST(object.get(vm.names.done)).as_bool()
+    };
+}
+
 void IteratorToArray::execute_impl(Bytecode::Interpreter& interpreter) const
 {
     auto& global_object = interpreter.global_object();
-    auto iterator_or_error = interpreter.accumulator().to_object(global_object);
+    auto iterator_object_or_error = interpreter.accumulator().to_object(global_object);
-    if (iterator_or_error.is_error())
+    if (iterator_object_or_error.is_error())
         return;
-    auto* iterator = iterator_or_error.release_value();
+    auto iterator = object_to_iterator(global_object, *iterator_object_or_error.release_value());
 
     auto* array = MUST(Array::create(global_object, 0));
     size_t index = 0;
 
     while (true) {
-        auto iterator_result_or_error = iterator_next(*iterator);
+        auto iterator_result_or_error = iterator_next(global_object, iterator);
         if (iterator_result_or_error.is_error())
             return;
         auto* iterator_result = iterator_result_or_error.release_value();
@@ -543,17 +566,17 @@ void GetIterator::execute_impl(Bytecode::Interpreter& interpreter) const
     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();
+    interpreter.accumulator() = iterator_to_object(interpreter.global_object(), iterator_or_error.release_value());
 }
 
 void IteratorNext::execute_impl(Bytecode::Interpreter& interpreter) const
 {
-    auto object_or_error = interpreter.accumulator().to_object(interpreter.global_object());
+    auto iterator_object_or_error = interpreter.accumulator().to_object(interpreter.global_object());
-    if (object_or_error.is_error())
+    if (iterator_object_or_error.is_error())
         return;
-    auto* object = object_or_error.release_value();
+    auto iterator = object_to_iterator(interpreter.global_object(), *iterator_object_or_error.release_value());
 
-    auto iterator_result_or_error = iterator_next(*object);
+    auto iterator_result_or_error = iterator_next(interpreter.global_object(), iterator);
     if (iterator_result_or_error.is_error())
         return;
     auto* iterator_result = iterator_result_or_error.release_value();

Userland/Libraries/LibJS/Runtime/ArrayConstructor.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2020, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2020-2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -114,10 +114,10 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
         while (true) {
             if (k >= MAX_ARRAY_LIKE_INDEX) {
                 auto error = vm.throw_completion<TypeError>(global_object, ErrorType::ArrayMaxSize);
-                return TRY(iterator_close(*iterator, move(error)));
+                return TRY(iterator_close(global_object, iterator, move(error)));
             }
 
-            auto* next = TRY(iterator_step(global_object, *iterator));
+            auto* next = TRY(iterator_step(global_object, iterator));
             if (!next) {
                 TRY(array->set(vm.names.length, Value(k), Object::ShouldThrowExceptions::Yes));
                 return array;
@@ -129,7 +129,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
             if (map_fn) {
                 auto mapped_value_or_error = vm.call(*map_fn, this_arg, next_value, Value(k));
                 if (mapped_value_or_error.is_error())
-                    return TRY(iterator_close(*iterator, mapped_value_or_error.release_error()));
+                    return TRY(iterator_close(global_object, iterator, mapped_value_or_error.release_error()));
                 mapped_value = mapped_value_or_error.release_value();
             } else {
                 mapped_value = next_value;
@@ -137,7 +137,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayConstructor::from)
 
             auto result_or_error = array->create_data_property_or_throw(k, mapped_value);
             if (result_or_error.is_error())
-                return TRY(iterator_close(*iterator, result_or_error.release_error()));
+                return TRY(iterator_close(global_object, iterator, result_or_error.release_error()));
 
             ++k;
         }

Userland/Libraries/LibJS/Runtime/AsyncFromSyncIterator.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021, David Tuin <davidot@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -10,16 +11,15 @@
 
 namespace JS {
 
-AsyncFromSyncIterator* AsyncFromSyncIterator::create(GlobalObject& global_object, Object* sync_iterator_record)
+AsyncFromSyncIterator* AsyncFromSyncIterator::create(GlobalObject& global_object, Iterator sync_iterator_record)
 {
     return global_object.heap().allocate<AsyncFromSyncIterator>(global_object, global_object, sync_iterator_record);
 }
 
-AsyncFromSyncIterator::AsyncFromSyncIterator(GlobalObject& global_object, Object* sync_iterator_record)
+AsyncFromSyncIterator::AsyncFromSyncIterator(GlobalObject& global_object, Iterator sync_iterator_record)
     : Object(*global_object.async_from_sync_iterator_prototype())
     , m_sync_iterator_record(sync_iterator_record)
 {
-    VERIFY(m_sync_iterator_record);
 }
 
 void AsyncFromSyncIterator::initialize(GlobalObject& global_object)
@@ -30,7 +30,8 @@ void AsyncFromSyncIterator::initialize(GlobalObject& global_object)
 void AsyncFromSyncIterator::visit_edges(Cell::Visitor& visitor)
 {
     Object::visit_edges(visitor);
-    visitor.visit(m_sync_iterator_record);
+    visitor.visit(m_sync_iterator_record.iterator);
+    visitor.visit(m_sync_iterator_record.next_method);
 }
 
 }

Userland/Libraries/LibJS/Runtime/AsyncFromSyncIterator.h

@@ -1,11 +1,13 @@
 /*
  * Copyright (c) 2021, David Tuin <davidot@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #pragma once
 
+#include <LibJS/Runtime/Iterator.h>
 #include <LibJS/Runtime/Object.h>
 
 namespace JS {
@@ -15,18 +17,19 @@ class AsyncFromSyncIterator final : public Object {
     JS_OBJECT(AsyncFromSyncIterator, Object);
 
 public:
-    static AsyncFromSyncIterator* create(GlobalObject&, Object* sync_iterator_record);
+    static AsyncFromSyncIterator* create(GlobalObject&, Iterator sync_iterator_record);
 
-    explicit AsyncFromSyncIterator(GlobalObject&, Object* sync_iterator_record);
+    explicit AsyncFromSyncIterator(GlobalObject&, Iterator sync_iterator_record);
     virtual void initialize(GlobalObject&) override;
     virtual ~AsyncFromSyncIterator() override = default;
 
     void visit_edges(Visitor& visitor) override;
 
-    Object& sync_iterator_record() const { return *m_sync_iterator_record; }
+    Iterator& sync_iterator_record() { return m_sync_iterator_record; }
+    Iterator const& sync_iterator_record() const { return m_sync_iterator_record; }
 
 private:
-    Object* m_sync_iterator_record { nullptr }; // [[SyncIteratorRecord]]
+    Iterator m_sync_iterator_record; // [[SyncIteratorRecord]]
 };
 
 }

Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021, David Tuin <davidot@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -30,8 +31,10 @@ void AsyncFromSyncIteratorPrototype::initialize(GlobalObject& global_object)
 }
 
 // 27.1.4.4 AsyncFromSyncIteratorContinuation ( result, promiseCapability ), https://tc39.es/ecma262/#sec-asyncfromsynciteratorcontinuation
-static ThrowCompletionOr<Object*> async_from_sync_iterator_continuation(VM& vm, GlobalObject& global_object, Object& result, PromiseCapability& promise_capability)
+static ThrowCompletionOr<Object*> async_from_sync_iterator_continuation(GlobalObject& global_object, Object& result, PromiseCapability& promise_capability)
 {
+    auto& vm = global_object.vm();
+
     // 1. Let done be IteratorComplete(result).
     // 2. IfAbruptRejectPromise(done, promiseCapability).
     auto done = TRY_OR_REJECT(vm, promise_capability, iterator_complete(global_object, result));
@@ -75,17 +78,18 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::next)
 
     // 4. Let syncIteratorRecord be O.[[SyncIteratorRecord]].
     auto& sync_iterator_record = this_object->sync_iterator_record();
+
     // 5. If value is present, then
     //     a. Let result be IteratorNext(syncIteratorRecord, value).
     // 6. Else,
     //     a. Let result be IteratorNext(syncIteratorRecord).
     // 7. IfAbruptRejectPromise(result, promiseCapability).
     auto* result = TRY_OR_REJECT(vm, promise_capability,
-        (vm.argument_count() > 0 ? iterator_next(sync_iterator_record, vm.argument(0))
+        (vm.argument_count() > 0 ? iterator_next(global_object, sync_iterator_record, vm.argument(0))
-                                 : iterator_next(sync_iterator_record)));
+                                 : iterator_next(global_object, sync_iterator_record)));
 
     // 8. Return ! AsyncFromSyncIteratorContinuation(result, promiseCapability).
-    return MUST(async_from_sync_iterator_continuation(vm, global_object, *result, promise_capability));
+    return MUST(async_from_sync_iterator_continuation(global_object, *result, promise_capability));
 }
 
 // 27.1.4.2.2 %AsyncFromSyncIteratorPrototype%.return ( [ value ] ), https://tc39.es/ecma262/#sec-%asyncfromsynciteratorprototype%.return
@@ -99,11 +103,11 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::return_)
     auto promise_capability = MUST(new_promise_capability(global_object, global_object.promise_constructor()));
 
     // 4. Let syncIterator be O.[[SyncIteratorRecord]].[[Iterator]].
-    auto& sync_iterator = this_object->sync_iterator_record();
+    auto* sync_iterator = this_object->sync_iterator_record().iterator;
 
     // 5. Let return be GetMethod(syncIterator, "return").
     // 6. IfAbruptRejectPromise(return, promiseCapability).
-    auto* return_method = TRY_OR_REJECT(vm, promise_capability, Value(&sync_iterator).get_method(global_object, vm.names.return_));
+    auto* return_method = TRY_OR_REJECT(vm, promise_capability, Value(sync_iterator).get_method(global_object, vm.names.return_));
 
     // 7. If return is undefined, then
     if (return_method == nullptr) {
@@ -123,8 +127,8 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::return_)
     //     a. Let result be Call(return, syncIterator).
     // 10. IfAbruptRejectPromise(result, promiseCapability).
     auto result = TRY_OR_REJECT(vm, promise_capability,
-        (vm.argument_count() > 0 ? call(global_object, return_method, &sync_iterator, vm.argument(0))
+        (vm.argument_count() > 0 ? call(global_object, return_method, sync_iterator, vm.argument(0))
-                                 : call(global_object, return_method, &sync_iterator)));
+                                 : call(global_object, return_method, sync_iterator)));
 
     // 11. If Type(result) is not Object, then
     if (!result.is_object()) {
@@ -136,7 +140,7 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::return_)
     }
 
     // 12. Return ! AsyncFromSyncIteratorContinuation(result, promiseCapability).
-    return MUST(async_from_sync_iterator_continuation(vm, global_object, result.as_object(), promise_capability));
+    return MUST(async_from_sync_iterator_continuation(global_object, result.as_object(), promise_capability));
 }
 
 // 27.1.4.2.3 %AsyncFromSyncIteratorPrototype%.throw ( [ value ] ), https://tc39.es/ecma262/#sec-%asyncfromsynciteratorprototype%.throw
@@ -150,11 +154,11 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::throw_)
     auto promise_capability = MUST(new_promise_capability(global_object, global_object.promise_constructor()));
 
     // 4. Let syncIterator be O.[[SyncIteratorRecord]].[[Iterator]].
-    auto& sync_iterator = this_object->sync_iterator_record();
+    auto* sync_iterator = this_object->sync_iterator_record().iterator;
 
     // 5. Let throw be GetMethod(syncIterator, "throw").
     // 6. IfAbruptRejectPromise(throw, promiseCapability).
-    auto* throw_method = TRY_OR_REJECT(vm, promise_capability, Value(&sync_iterator).get_method(global_object, vm.names.throw_));
+    auto* throw_method = TRY_OR_REJECT(vm, promise_capability, Value(sync_iterator).get_method(global_object, vm.names.throw_));
 
     // 7. If throw is undefined, then
     if (throw_method == nullptr) {
@@ -169,8 +173,8 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::throw_)
     //     a. Let result be Call(throw, syncIterator).
     // 10. IfAbruptRejectPromise(result, promiseCapability).
     auto result = TRY_OR_REJECT(vm, promise_capability,
-        (vm.argument_count() > 0 ? call(global_object, throw_method, &sync_iterator, vm.argument(0))
+        (vm.argument_count() > 0 ? call(global_object, throw_method, sync_iterator, vm.argument(0))
-                                 : call(global_object, throw_method, &sync_iterator)));
+                                 : call(global_object, throw_method, sync_iterator)));
 
     // 11. If Type(result) is not Object, then
     if (!result.is_object()) {
@@ -183,21 +187,26 @@ JS_DEFINE_NATIVE_FUNCTION(AsyncFromSyncIteratorPrototype::throw_)
     }
 
     // 12. Return ! AsyncFromSyncIteratorContinuation(result, promiseCapability).
-    return MUST(async_from_sync_iterator_continuation(vm, global_object, result.as_object(), promise_capability));
+    return MUST(async_from_sync_iterator_continuation(global_object, result.as_object(), promise_capability));
 }
 
 // 27.1.4.1 CreateAsyncFromSyncIterator ( syncIteratorRecord ), https://tc39.es/ecma262/#sec-createasyncfromsynciterator
-ThrowCompletionOr<Object*> create_async_from_sync_iterator(GlobalObject& global_object, Object& sync_iterator_record)
+ThrowCompletionOr<Iterator> create_async_from_sync_iterator(GlobalObject& global_object, Iterator sync_iterator_record)
 {
+    auto& vm = global_object.vm();
+
     // 1. Let asyncIterator be ! OrdinaryObjectCreate(%AsyncFromSyncIteratorPrototype%, « [[SyncIteratorRecord]] »).
     // 2. Set asyncIterator.[[SyncIteratorRecord]] to syncIteratorRecord.
+    auto* async_iterator = AsyncFromSyncIterator::create(global_object, sync_iterator_record);
+
     // 3. Let nextMethod be ! Get(asyncIterator, "next").
+    auto next_method = MUST(async_iterator->get(vm.names.next));
+
     // 4. Let iteratorRecord be the Record { [[Iterator]]: asyncIterator, [[NextMethod]]: nextMethod, [[Done]]: false }.
+    auto iterator_record = Iterator { .iterator = async_iterator, .next_method = next_method, .done = false };
+
     // 5. Return iteratorRecord.
-    // FIXME: Use actual iterator records instead of objects.
+    return iterator_record;
-
-    // Note: AsyncFromSyncIterator is an object with the extra slot SyncIteratorRecord.
-    return AsyncFromSyncIterator::create(global_object, &sync_iterator_record);
 }
 
 }

Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2021, David Tuin <davidot@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -8,6 +9,7 @@
 
 #include <LibJS/Runtime/AsyncFromSyncIterator.h>
 #include <LibJS/Runtime/Completion.h>
+#include <LibJS/Runtime/Iterator.h>
 #include <LibJS/Runtime/PrototypeObject.h>
 
 namespace JS {
@@ -27,6 +29,6 @@ private:
     JS_DECLARE_NATIVE_FUNCTION(throw_);
 };
 
-ThrowCompletionOr<Object*> create_async_from_sync_iterator(GlobalObject&, Object& sync_iterator);
+ThrowCompletionOr<Iterator> create_async_from_sync_iterator(GlobalObject&, Iterator sync_iterator);
 
 }

Userland/Libraries/LibJS/Runtime/Intl/ListFormat.cpp

@@ -276,7 +276,7 @@ ThrowCompletionOr<Vector<String>> string_list_from_iterable(GlobalObject& global
     }
 
     // 2. Let iteratorRecord be ? GetIterator(iterable).
-    auto* iterator_record = TRY(get_iterator(global_object, iterable));
+    auto iterator_record = TRY(get_iterator(global_object, iterable));
 
     // 3. Let list be a new empty List.
     Vector<String> list;
@@ -287,7 +287,7 @@ ThrowCompletionOr<Vector<String>> string_list_from_iterable(GlobalObject& global
     // 5. Repeat, while next is not false,
     do {
         // a. Set next to ? IteratorStep(iteratorRecord).
-        next = TRY(iterator_step(global_object, *iterator_record));
+        next = TRY(iterator_step(global_object, iterator_record));
 
         // b. If next is not false, then
         if (next != nullptr) {
@@ -300,7 +300,7 @@ ThrowCompletionOr<Vector<String>> string_list_from_iterable(GlobalObject& global
                 auto error = vm.throw_completion<TypeError>(global_object, ErrorType::NotAString, next_value);
 
                 // 2. Return ? IteratorClose(iteratorRecord, error).
-                return iterator_close(*iterator_record, move(error));
+                return iterator_close(global_object, iterator_record, move(error));
             }
 
             // iii. Append nextValue to the end of the List list.

Userland/Libraries/LibJS/Runtime/Iterator.h

@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <LibJS/Runtime/Object.h>
+#include <LibJS/Runtime/Value.h>
+
+namespace JS {
+
+// Iterator Record
+struct Iterator {
+    Object* iterator { nullptr }; // [[Iterator]]
+    Value next_method;            // [[NextMethod]]
+    bool done { false };          // [[Done]]
+};
+
+}

Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -15,53 +16,81 @@
 namespace JS {
 
 // 7.4.1 GetIterator ( obj [ , hint [ , method ] ] ), https://tc39.es/ecma262/#sec-getiterator
-ThrowCompletionOr<Object*> get_iterator(GlobalObject& global_object, Value value, IteratorHint hint, Optional<Value> method)
+ThrowCompletionOr<Iterator> get_iterator(GlobalObject& global_object, Value value, IteratorHint hint, Optional<Value> method)
 {
     auto& vm = global_object.vm();
+
+    // 1. If hint is not present, set hint to sync.
+
+    // 2. If method is not present, then
     if (!method.has_value()) {
+        // a. If hint is async, then
         if (hint == IteratorHint::Async) {
+            // i. Set method to ? GetMethod(obj, @@asyncIterator).
             auto* async_method = TRY(value.get_method(global_object, *vm.well_known_symbol_async_iterator()));
+
+            // ii. If method is undefined, then
             if (async_method == nullptr) {
+                // 1. Let syncMethod be ? GetMethod(obj, @@iterator).
                 auto* sync_method = TRY(value.get_method(global_object, *vm.well_known_symbol_iterator()));
-                auto* sync_iterator_record = TRY(get_iterator(global_object, value, IteratorHint::Sync, sync_method));
+
-                return TRY(create_async_from_sync_iterator(global_object, *sync_iterator_record));
+                // 2. Let syncIteratorRecord be ? GetIterator(obj, sync, syncMethod).
+                auto sync_iterator_record = TRY(get_iterator(global_object, value, IteratorHint::Sync, sync_method));
+
+                // 3. Return ! CreateAsyncFromSyncIterator(syncIteratorRecord).
+                return MUST(create_async_from_sync_iterator(global_object, sync_iterator_record));
             }
+
             method = Value(async_method);
-        } else {
+        }
+        // b. Otherwise, set method to ? GetMethod(obj, @@iterator).
+        else {
             method = TRY(value.get_method(global_object, *vm.well_known_symbol_iterator()));
         }
     }
 
+    // NOTE: Additional type check to produce a better error message than Call().
     if (!method->is_function())
         return vm.throw_completion<TypeError>(global_object, ErrorType::NotIterable, value.to_string_without_side_effects());
 
-    auto iterator = TRY(vm.call(method->as_function(), value));
+    // 3. Let iterator be ? Call(method, obj).
+    auto iterator = TRY(call(global_object, *method, value));
+
+    // 4. If Type(iterator) is not Object, throw a TypeError exception.
     if (!iterator.is_object())
         return vm.throw_completion<TypeError>(global_object, ErrorType::NotIterable, value.to_string_without_side_effects());
 
-    return &iterator.as_object();
+    // 5. Let nextMethod be ? GetV(iterator, "next").
+    auto next_method = TRY(iterator.get(global_object, vm.names.next));
+
+    // 6. Let iteratorRecord be the Record { [[Iterator]]: iterator, [[NextMethod]]: nextMethod, [[Done]]: false }.
+    auto iterator_record = Iterator { .iterator = &iterator.as_object(), .next_method = next_method, .done = false };
+
+    // 7. Return iteratorRecord.
+    return iterator_record;
 }
 
 // 7.4.2 IteratorNext ( iteratorRecord [ , value ] ), https://tc39.es/ecma262/#sec-iteratornext
-ThrowCompletionOr<Object*> iterator_next(Object& iterator, Optional<Value> value)
+ThrowCompletionOr<Object*> iterator_next(GlobalObject& global_object, Iterator const& iterator_record, Optional<Value> value)
 {
-    // FIXME: Implement using iterator records, not ordinary objects
+    auto& vm = global_object.vm();
-    auto& vm = iterator.vm();
-    auto& global_object = iterator.global_object();
-
-    auto next_method = TRY(iterator.get(vm.names.next));
-    if (!next_method.is_function())
-        return vm.throw_completion<TypeError>(global_object, ErrorType::IterableNextNotAFunction);
 
     Value result;
-    if (!value.has_value())
-        result = TRY(vm.call(next_method.as_function(), &iterator));
-    else
-        result = TRY(vm.call(next_method.as_function(), &iterator, *value));
 
+    // 1. If value is not present, then
+    if (!value.has_value()) {
+        // a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]]).
+        result = TRY(call(global_object, iterator_record.next_method, iterator_record.iterator));
+    } else {
+        // a. Let result be ? Call(iteratorRecord.[[NextMethod]], iteratorRecord.[[Iterator]], « value »).
+        result = TRY(call(global_object, iterator_record.next_method, iterator_record.iterator, *value));
+    }
+
+    // 3. If Type(result) is not Object, throw a TypeError exception.
     if (!result.is_object())
         return vm.throw_completion<TypeError>(global_object, ErrorType::IterableNextBadReturn);
 
+    // 4. Return result.
     return &result.as_object();
 }
 
@@ -84,53 +113,60 @@ ThrowCompletionOr<Value> iterator_value(GlobalObject& global_object, Object& ite
 }
 
 // 7.4.5 IteratorStep ( iteratorRecord ), https://tc39.es/ecma262/#sec-iteratorstep
-ThrowCompletionOr<Object*> iterator_step(GlobalObject& global_object, Object& iterator)
+ThrowCompletionOr<Object*> iterator_step(GlobalObject& global_object, Iterator const& iterator_record)
 {
-    auto* result = TRY(iterator_next(iterator));
+    // 1. Let result be ? IteratorNext(iteratorRecord).
+    auto* result = TRY(iterator_next(global_object, iterator_record));
+
+    // 2. Let done be ? IteratorComplete(result).
     auto done = TRY(iterator_complete(global_object, *result));
 
+    // 3. If done is true, return false.
     if (done)
         return nullptr;
 
+    // 4. Return result.
     return result;
 }
 
 // 7.4.6 IteratorClose ( iteratorRecord, completion ), https://tc39.es/ecma262/#sec-iteratorclose
 // 7.4.8 AsyncIteratorClose ( iteratorRecord, completion ), https://tc39.es/ecma262/#sec-asynciteratorclose
 // NOTE: These only differ in that async awaits the inner value after the call.
-static Completion iterator_close_(Object& iterator, Completion completion, IteratorHint iterator_hint)
+static Completion iterator_close_impl(GlobalObject& global_object, Iterator const& iterator_record, Completion completion, IteratorHint iterator_hint)
 {
-    auto& vm = iterator.vm();
+    auto& vm = global_object.vm();
-    auto& global_object = iterator.global_object();
+
+    // 1. Assert: Type(iteratorRecord.[[Iterator]]) is Object.
+
+    // 2. Let iterator be iteratorRecord.[[Iterator]].
+    auto* iterator = iterator_record.iterator;
 
     // The callers of iterator_close() are often in an exceptional state.
     // Temporarily clear that exception for invocation(s) to Call.
     TemporaryClearException clear_exception(vm);
 
     // 3. Let innerResult be GetMethod(iterator, "return").
-    auto inner_result_or_error = Value(&iterator).get_method(global_object, vm.names.return_);
+    auto inner_result = ThrowCompletionOr<Value> { js_undefined() };
-    Value inner_result;
+    auto get_method_result = Value(iterator).get_method(global_object, vm.names.return_);
+    if (get_method_result.is_error())
+        inner_result = get_method_result.release_error();
 
     // 4. If innerResult.[[Type]] is normal, then
-    if (!inner_result_or_error.is_error()) {
+    if (!inner_result.is_error()) {
         // a. Let return be innerResult.[[Value]].
-        auto* return_method = inner_result_or_error.release_value();
+        auto* return_method = get_method_result.value();
 
         // b. If return is undefined, return Completion(completion).
         if (!return_method)
             return completion;
 
         // c. Set innerResult to Call(return, iterator).
-        auto result_or_error = vm.call(*return_method, &iterator);
+        inner_result = call(global_object, return_method, iterator);
-        if (result_or_error.is_error()) {
+
-            inner_result_or_error = result_or_error.release_error();
+        // Note: If this is AsyncIteratorClose perform one extra step.
-        } else {
+        if (iterator_hint == IteratorHint::Async && !inner_result.is_error()) {
-            inner_result = result_or_error.release_value();
+            // d. If innerResult.[[Type]] is normal, set innerResult to Await(innerResult.[[Value]]).
-            // Note: If this is AsyncIteratorClose perform one extra step.
+            inner_result = await(global_object, inner_result.value());
-            if (iterator_hint == IteratorHint::Async) {
-                // d. If innerResult.[[Type]] is normal, set innerResult to Await(innerResult.[[Value]]).
-                inner_result = TRY(await(global_object, inner_result));
-            }
         }
     }
 
@@ -139,11 +175,11 @@ static Completion iterator_close_(Object& iterator, Completion completion, Itera
         return completion;
 
     // 6. If innerResult.[[Type]] is throw, return Completion(innerResult).
-    if (inner_result_or_error.is_error())
+    if (inner_result.is_throw_completion())
-        return inner_result_or_error.release_error();
+        return inner_result;
 
     // 7. If Type(innerResult.[[Value]]) is not Object, throw a TypeError exception.
-    if (!inner_result.is_object())
+    if (!inner_result.value().is_object())
         return vm.throw_completion<TypeError>(global_object, ErrorType::IterableReturnBadReturn);
 
     // 8. Return Completion(completion).
@@ -151,24 +187,32 @@ static Completion iterator_close_(Object& iterator, Completion completion, Itera
 }
 
 // 7.4.6 IteratorClose ( iteratorRecord, completion ), https://tc39.es/ecma262/#sec-iteratorclose
-Completion iterator_close(Object& iterator, Completion completion)
+Completion iterator_close(GlobalObject& global_object, Iterator const& iterator_record, Completion completion)
 {
-    return iterator_close_(iterator, move(completion), IteratorHint::Sync);
+    return iterator_close_impl(global_object, iterator_record, move(completion), IteratorHint::Sync);
 }
 
 // 7.4.8 AsyncIteratorClose ( iteratorRecord, completion ), https://tc39.es/ecma262/#sec-asynciteratorclose
-Completion async_iterator_close(Object& iterator, Completion completion)
+Completion async_iterator_close(GlobalObject& global_object, Iterator const& iterator_record, Completion completion)
 {
-    return iterator_close_(iterator, move(completion), IteratorHint::Async);
+    return iterator_close_impl(global_object, iterator_record, move(completion), IteratorHint::Async);
 }
 
 // 7.4.9 CreateIterResultObject ( value, done ), https://tc39.es/ecma262/#sec-createiterresultobject
 Object* create_iterator_result_object(GlobalObject& global_object, Value value, bool done)
 {
     auto& vm = global_object.vm();
+
+    // 1. Let obj be ! OrdinaryObjectCreate(%Object.prototype%).
     auto* object = Object::create(global_object, global_object.object_prototype());
+
+    // 2. Perform ! CreateDataPropertyOrThrow(obj, "value", value).
     MUST(object->create_data_property_or_throw(vm.names.value, value));
+
+    // 3. Perform ! CreateDataPropertyOrThrow(obj, "done", done).
     MUST(object->create_data_property_or_throw(vm.names.done, Value(done)));
+
+    // 4. Return obj.
     return object;
 }
 
@@ -188,19 +232,20 @@ ThrowCompletionOr<MarkedValueList> iterable_to_list(GlobalObject& global_object,
     return { move(values) };
 }
 
+// Non-standard
 Completion get_iterator_values(GlobalObject& global_object, Value iterable, IteratorValueCallback callback, Optional<Value> method)
 {
-    auto* iterator = TRY(get_iterator(global_object, iterable, IteratorHint::Sync, move(method)));
+    auto iterator_record = TRY(get_iterator(global_object, iterable, IteratorHint::Sync, move(method)));
 
     while (true) {
-        auto* next_object = TRY(iterator_step(global_object, *iterator));
+        auto* next_object = TRY(iterator_step(global_object, iterator_record));
         if (!next_object)
             return {};
 
         auto next_value = TRY(iterator_value(global_object, *next_object));
 
         if (auto completion = callback(next_value); completion.has_value())
-            return iterator_close(*iterator, completion.release_value());
+            return iterator_close(global_object, iterator_record, completion.release_value());
     }
 }
 

Userland/Libraries/LibJS/Runtime/IteratorOperations.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Matthew Olsson <mattco@serenityos.org>
+ * Copyright (c) 2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -9,6 +10,7 @@
 #include <AK/Function.h>
 #include <AK/Optional.h>
 #include <LibJS/Runtime/Completion.h>
+#include <LibJS/Runtime/Iterator.h>
 #include <LibJS/Runtime/Object.h>
 
 namespace JS {
@@ -20,17 +22,17 @@ enum class IteratorHint {
     Async,
 };
 
-ThrowCompletionOr<Object*> get_iterator(GlobalObject&, Value value, IteratorHint hint = IteratorHint::Sync, Optional<Value> method = {});
+ThrowCompletionOr<Iterator> get_iterator(GlobalObject&, Value, IteratorHint = IteratorHint::Sync, Optional<Value> method = {});
-ThrowCompletionOr<Object*> iterator_next(Object& iterator, Optional<Value> value = {});
+ThrowCompletionOr<Object*> iterator_next(GlobalObject&, Iterator const&, Optional<Value> = {});
-ThrowCompletionOr<Object*> iterator_step(GlobalObject&, Object& iterator);
+ThrowCompletionOr<Object*> iterator_step(GlobalObject&, Iterator const&);
 ThrowCompletionOr<bool> iterator_complete(GlobalObject&, Object& iterator_result);
 ThrowCompletionOr<Value> iterator_value(GlobalObject&, Object& iterator_result);
-Completion iterator_close(Object& iterator, Completion completion);
+Completion iterator_close(GlobalObject&, Iterator const&, Completion);
-Completion async_iterator_close(Object& iterator, Completion completion);
+Completion async_iterator_close(GlobalObject&, Iterator const&, Completion);
-Object* create_iterator_result_object(GlobalObject&, Value value, bool done);
+Object* create_iterator_result_object(GlobalObject&, Value, bool done);
 ThrowCompletionOr<MarkedValueList> iterable_to_list(GlobalObject&, Value iterable, Optional<Value> method = {});
 
 using IteratorValueCallback = Function<Optional<Completion>(Value)>;
-Completion get_iterator_values(GlobalObject& global_object, Value iterable, IteratorValueCallback callback, Optional<Value> method = {});
+Completion get_iterator_values(GlobalObject&, Value iterable, IteratorValueCallback callback, Optional<Value> method = {});
 
 }

Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -38,30 +38,10 @@ static ThrowCompletionOr<Value> get_promise_resolve(GlobalObject& global_object,
     return promise_resolve;
 }
 
-static bool iterator_record_is_complete(GlobalObject& global_object, Object& iterator_record)
-{
-    auto& vm = global_object.vm();
-
-    // FIXME: Create a native iterator structure with the [[Done]] internal slot. For now, temporarily clear
-    //        the exception so we can access the "done" property on the iterator object.
-    TemporaryClearException clear_exception(vm);
-    return MUST(iterator_complete(global_object, iterator_record));
-}
-
-static void set_iterator_record_complete(GlobalObject& global_object, Object& iterator_record)
-{
-    auto& vm = global_object.vm();
-
-    // FIXME: Create a native iterator structure with the [[Done]] internal slot. For now, temporarily clear
-    //        the exception so we can access the "done" property on the iterator object.
-    TemporaryClearException clear_exception(vm);
-    MUST(iterator_record.set(vm.names.done, Value(true), Object::ShouldThrowExceptions::No));
-}
-
 using EndOfElementsCallback = Function<ThrowCompletionOr<Value>(PromiseValueList&)>;
 using InvokeElementFunctionCallback = Function<ThrowCompletionOr<Value>(PromiseValueList&, RemainingElements&, Value, size_t)>;
 
-static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_object, Object& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve, EndOfElementsCallback end_of_list, InvokeElementFunctionCallback invoke_element_function)
+static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_object, Iterator& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve, EndOfElementsCallback end_of_list, InvokeElementFunctionCallback invoke_element_function)
 {
     auto& vm = global_object.vm();
 
@@ -85,7 +65,7 @@ static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_obje
         // b. If next is an abrupt completion, set iteratorRecord.[[Done]] to true.
         // c. ReturnIfAbrupt(next).
         if (next_or_error.is_throw_completion()) {
-            set_iterator_record_complete(global_object, iterator_record);
+            iterator_record.done = true;
             return next_or_error.release_error();
         }
         auto* next = next_or_error.release_value();
@@ -93,7 +73,7 @@ static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_obje
         // d. If next is false, then
         if (!next) {
             // i. Set iteratorRecord.[[Done]] to true.
-            set_iterator_record_complete(global_object, iterator_record);
+            iterator_record.done = true;
 
             // ii. Set remainingElementsCount.[[Value]] to remainingElementsCount.[[Value]] - 1.
             // iii. If remainingElementsCount.[[Value]] is 0, then
@@ -112,7 +92,7 @@ static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_obje
         // f. If nextValue is an abrupt completion, set iteratorRecord.[[Done]] to true.
         // g. ReturnIfAbrupt(nextValue).
         if (next_value_or_error.is_throw_completion()) {
-            set_iterator_record_complete(global_object, iterator_record);
+            iterator_record.done = true;
             return next_value_or_error.release_error();
         }
         auto next_value = next_value_or_error.release_value();
@@ -137,7 +117,7 @@ static ThrowCompletionOr<Value> perform_promise_common(GlobalObject& global_obje
 }
 
 // 27.2.4.1.2 PerformPromiseAll ( iteratorRecord, constructor, resultCapability, promiseResolve ), https://tc39.es/ecma262/#sec-performpromiseall
-static ThrowCompletionOr<Value> perform_promise_all(GlobalObject& global_object, Object& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
+static ThrowCompletionOr<Value> perform_promise_all(GlobalObject& global_object, Iterator& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
 {
     auto& vm = global_object.vm();
 
@@ -171,7 +151,7 @@ static ThrowCompletionOr<Value> perform_promise_all(GlobalObject& global_object,
 }
 
 // 27.2.4.2.1 PerformPromiseAllSettled ( iteratorRecord, constructor, resultCapability, promiseResolve ), https://tc39.es/ecma262/#sec-performpromiseallsettled
-static ThrowCompletionOr<Value> perform_promise_all_settled(GlobalObject& global_object, Object& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
+static ThrowCompletionOr<Value> perform_promise_all_settled(GlobalObject& global_object, Iterator& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
 {
     auto& vm = global_object.vm();
 
@@ -214,7 +194,7 @@ static ThrowCompletionOr<Value> perform_promise_all_settled(GlobalObject& global
 }
 
 // 27.2.4.3.1 PerformPromiseAny ( iteratorRecord, constructor, resultCapability, promiseResolve ), https://tc39.es/ecma262/#sec-performpromiseany
-static ThrowCompletionOr<Value> perform_promise_any(GlobalObject& global_object, Object& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
+static ThrowCompletionOr<Value> perform_promise_any(GlobalObject& global_object, Iterator& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
 {
     auto& vm = global_object.vm();
 
@@ -250,7 +230,7 @@ static ThrowCompletionOr<Value> perform_promise_any(GlobalObject& global_object,
 }
 
 // 27.2.4.5.1 PerformPromiseRace ( iteratorRecord, constructor, resultCapability, promiseResolve ), https://tc39.es/ecma262/#sec-performpromiserace
-static ThrowCompletionOr<Value> perform_promise_race(GlobalObject& global_object, Object& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
+static ThrowCompletionOr<Value> perform_promise_race(GlobalObject& global_object, Iterator& iterator_record, Value constructor, PromiseCapability result_capability, Value promise_resolve)
 {
     auto& vm = global_object.vm();
 
@@ -353,16 +333,16 @@ JS_DEFINE_NATIVE_FUNCTION(PromiseConstructor::all)
 
     // 5. Let iteratorRecord be GetIterator(iterable).
     // 6. IfAbruptRejectPromise(iteratorRecord, promiseCapability).
-    auto* iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
+    auto iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
 
     // 7. Let result be PerformPromiseAll(iteratorRecord, C, promiseCapability, promiseResolve).
-    auto result = perform_promise_all(global_object, *iterator_record, constructor, promise_capability, promise_resolve);
+    auto result = perform_promise_all(global_object, iterator_record, constructor, promise_capability, promise_resolve);
 
     // 8. If result is an abrupt completion, then
     if (result.is_error()) {
         // a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
-        if (!iterator_record_is_complete(global_object, *iterator_record))
+        if (!iterator_record.done)
-            result = iterator_close(*iterator_record, result.release_error());
+            result = iterator_close(global_object, iterator_record, result.release_error());
 
         // b. IfAbruptRejectPromise(result, promiseCapability).
         TRY_OR_REJECT(vm, promise_capability, result);
@@ -387,16 +367,16 @@ JS_DEFINE_NATIVE_FUNCTION(PromiseConstructor::all_settled)
 
     // 5. Let iteratorRecord be GetIterator(iterable).
     // 6. IfAbruptRejectPromise(iteratorRecord, promiseCapability).
-    auto* iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
+    auto iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
 
     // 7. Let result be PerformPromiseAllSettled(iteratorRecord, C, promiseCapability, promiseResolve).
-    auto result = perform_promise_all_settled(global_object, *iterator_record, constructor, promise_capability, promise_resolve);
+    auto result = perform_promise_all_settled(global_object, iterator_record, constructor, promise_capability, promise_resolve);
 
     // 8. If result is an abrupt completion, then
     if (result.is_error()) {
         // a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
-        if (!iterator_record_is_complete(global_object, *iterator_record))
+        if (!iterator_record.done)
-            result = iterator_close(*iterator_record, result.release_error());
+            result = iterator_close(global_object, iterator_record, result.release_error());
 
         // b. IfAbruptRejectPromise(result, promiseCapability).
         TRY_OR_REJECT(vm, promise_capability, result);
@@ -421,16 +401,16 @@ JS_DEFINE_NATIVE_FUNCTION(PromiseConstructor::any)
 
     // 5. Let iteratorRecord be GetIterator(iterable).
     // 6. IfAbruptRejectPromise(iteratorRecord, promiseCapability).
-    auto* iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
+    auto iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
 
     // 7. Let result be PerformPromiseAny(iteratorRecord, C, promiseCapability, promiseResolve).
-    auto result = perform_promise_any(global_object, *iterator_record, constructor, promise_capability, promise_resolve);
+    auto result = perform_promise_any(global_object, iterator_record, constructor, promise_capability, promise_resolve);
 
     // 8. If result is an abrupt completion, then
     if (result.is_error()) {
         // a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
-        if (!iterator_record_is_complete(global_object, *iterator_record))
+        if (!iterator_record.done)
-            result = iterator_close(*iterator_record, result.release_error());
+            result = iterator_close(global_object, iterator_record, result.release_error());
 
         // b. IfAbruptRejectPromise(result, promiseCapability).
         TRY_OR_REJECT(vm, promise_capability, result);
@@ -455,16 +435,16 @@ JS_DEFINE_NATIVE_FUNCTION(PromiseConstructor::race)
 
     // 5. Let iteratorRecord be GetIterator(iterable).
     // 6. IfAbruptRejectPromise(iteratorRecord, promiseCapability).
-    auto* iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
+    auto iterator_record = TRY_OR_REJECT(vm, promise_capability, get_iterator(global_object, vm.argument(0)));
 
     // 7. Let result be PerformPromiseRace(iteratorRecord, C, promiseCapability, promiseResolve).
-    auto result = perform_promise_race(global_object, *iterator_record, constructor, promise_capability, promise_resolve);
+    auto result = perform_promise_race(global_object, iterator_record, constructor, promise_capability, promise_resolve);
 
     // 8. If result is an abrupt completion, then
     if (result.is_error()) {
         // a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
-        if (!iterator_record_is_complete(global_object, *iterator_record))
+        if (!iterator_record.done)
-            result = iterator_close(*iterator_record, result.release_error());
+            result = iterator_close(global_object, iterator_record, result.release_error());
 
         // b. IfAbruptRejectPromise(result, promiseCapability).
         TRY_OR_REJECT(vm, promise_capability, result);

Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
- * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2021, Luke Wilde <lukew@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -54,7 +54,7 @@ ThrowCompletionOr<MarkedValueList> iterable_to_list_of_type(GlobalObject& global
     // 4. Repeat, while next is not false,
     while (next) {
         // a. Set next to ? IteratorStep(iteratorRecord).
-        auto* iterator_result = TRY(iterator_step(global_object, *iterator_record));
+        auto* iterator_result = TRY(iterator_step(global_object, iterator_record));
         next = iterator_result;
 
         // b. If next is not false, then
@@ -66,7 +66,7 @@ ThrowCompletionOr<MarkedValueList> iterable_to_list_of_type(GlobalObject& global
                 // 1. Let completion be ThrowCompletion(a newly created TypeError object).
                 auto completion = vm.throw_completion<TypeError>(global_object, ErrorType::IterableToListOfTypeInvalidValue, next_value.to_string_without_side_effects());
                 // 2. Return ? IteratorClose(iteratorRecord, completion).
-                return iterator_close(*iterator_record, move(completion));
+                return iterator_close(global_object, iterator_record, move(completion));
             }
             // iii. Append nextValue to the end of the List values.
             values.append(next_value);

Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -486,7 +486,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
     VERIFY(calendar->identifier() == "iso8601"sv);
 
     // 4. Let iteratorRecord be ? Getiterator(fields, sync).
-    auto* iterator_record = TRY(get_iterator(global_object, fields, IteratorHint::Sync));
+    auto iterator_record = TRY(get_iterator(global_object, fields, IteratorHint::Sync));
 
     // 5. Let fieldNames be a new empty List.
     auto field_names = MarkedValueList { vm.heap() };
@@ -495,7 +495,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
     // 7. Repeat, while next is not false,
     while (true) {
         // a. Set next to ? IteratorStep(iteratorRecord).
-        auto* next = TRY(iterator_step(global_object, *iterator_record));
+        auto* next = TRY(iterator_step(global_object, iterator_record));
 
         // b. If next is not false, then
         if (!next)
@@ -510,7 +510,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<TypeError>(global_object, ErrorType::TemporalInvalidCalendarFieldValue, next_value.to_string_without_side_effects());
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(*iterator_record, move(completion)));
+            return TRY(iterator_close(global_object, iterator_record, move(completion)));
         }
 
         // iii. If fieldNames contains nextValue, then
@@ -519,7 +519,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<RangeError>(global_object, ErrorType::TemporalDuplicateCalendarField, next_value.as_string().string());
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(*iterator_record, move(completion)));
+            return TRY(iterator_close(global_object, iterator_record, move(completion)));
         }
 
         // iv. If nextValue is not one of "year", "month", "monthCode", "day", "hour", "minute", "second", "millisecond", "microsecond", "nanosecond", then
@@ -528,7 +528,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             auto completion = vm.throw_completion<RangeError>(global_object, ErrorType::TemporalInvalidCalendarFieldName, next_value.as_string().string());
 
             // 2. Return ? IteratorClose(iteratorRecord, completion).
-            return TRY(iterator_close(*iterator_record, move(completion)));
+            return TRY(iterator_close(global_object, iterator_record, move(completion)));
         }
 
         // v. Append nextValue to the end of the List fieldNames.

Userland/Libraries/LibJS/Runtime/Temporal/TimeZone.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2021-2022, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -612,7 +612,7 @@ ThrowCompletionOr<MarkedValueList> get_possible_instants_for(GlobalObject& globa
     auto possible_instants = TRY(time_zone.invoke(global_object, vm.names.getPossibleInstantsFor, &date_time));
 
     // 3. Let iteratorRecord be ? GetIterator(possibleInstants, sync).
-    auto* iterator = TRY(get_iterator(global_object, possible_instants, IteratorHint::Sync));
+    auto iterator = TRY(get_iterator(global_object, possible_instants, IteratorHint::Sync));
 
     // 4. Let list be a new empty List.
     auto list = MarkedValueList { vm.heap() };
@@ -623,7 +623,7 @@ ThrowCompletionOr<MarkedValueList> get_possible_instants_for(GlobalObject& globa
     // 6. Repeat, while next is not false,
     do {
         // a. Set next to ? IteratorStep(iteratorRecord).
-        next = TRY(iterator_step(global_object, *iterator));
+        next = TRY(iterator_step(global_object, iterator));
 
         // b. If next is not false, then
         if (next) {
@@ -636,7 +636,7 @@ ThrowCompletionOr<MarkedValueList> get_possible_instants_for(GlobalObject& globa
                 auto completion = vm.throw_completion<TypeError>(global_object, ErrorType::NotAnObjectOfType, "Temporal.Instant");
 
                 // 2. Return ? IteratorClose(iteratorRecord, completion).
-                return iterator_close(*iterator, move(completion));
+                return iterator_close(global_object, iterator, move(completion));
             }
 
             // iii. Append nextValue to the end of the List list.

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -197,18 +197,17 @@ ThrowCompletionOr<void> VM::binding_initialization(NonnullRefPtr<BindingPattern>
     // BindingPattern : ArrayBindingPattern
     else {
         // 1. Let iteratorRecord be ? GetIterator(value).
-        auto* iterator = TRY(get_iterator(global_object, value));
+        auto iterator_record = TRY(get_iterator(global_object, value));
-        auto iterator_done = false;
 
         // 2. Let result be IteratorBindingInitialization of ArrayBindingPattern with arguments iteratorRecord and environment.
-        auto result = iterator_binding_initialization(*target, iterator, iterator_done, environment, global_object);
+        auto result = iterator_binding_initialization(*target, iterator_record, environment, global_object);
 
         // 3. If iteratorRecord.[[Done]] is false, return ? IteratorClose(iteratorRecord, result).
-        if (!iterator_done) {
+        if (!iterator_record.done) {
             // iterator_close() always returns a Completion, which ThrowCompletionOr will interpret as a throw
             // completion. So only return the result of iterator_close() if it is indeed a throw completion.
             auto completion = result.is_throw_completion() ? result.release_error() : normal_completion({});
-            if (completion = iterator_close(*iterator, move(completion)); completion.is_error())
+            if (completion = iterator_close(global_object, iterator_record, move(completion)); completion.is_error())
                 return completion.release_error();
         }
 
@@ -311,7 +310,7 @@ ThrowCompletionOr<void> VM::property_binding_initialization(BindingPattern const
 
 // 13.15.5.5 Runtime Semantics: IteratorDestructuringAssignmentEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-iteratordestructuringassignmentevaluation
 // 8.5.3 Runtime Semantics: IteratorBindingInitialization, https://tc39.es/ecma262/#sec-runtime-semantics-iteratorbindinginitialization
-ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const& binding, Object* iterator, bool& iterator_done, Environment* environment, GlobalObject& global_object)
+ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const& binding, Iterator& iterator_record, Environment* environment, GlobalObject& global_object)
 {
     // FIXME: this method is nearly identical to destructuring assignment!
     for (size_t i = 0; i < binding.entries.size(); i++) {
@@ -328,51 +327,97 @@ ThrowCompletionOr<void> VM::iterator_binding_initialization(BindingPattern const
                 return TRY(member_expression->to_reference(interpreter(), global_object));
             }));
 
+        // BindingRestElement : ... BindingIdentifier
+        // BindingRestElement : ... BindingPattern
         if (entry.is_rest) {
             VERIFY(i == binding.entries.size() - 1);
 
+            // 2. Let A be ! ArrayCreate(0).
             auto* array = MUST(Array::create(global_object, 0));
-            while (!iterator_done) {
-                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());
-                }
-                auto* next_object = next_object_or_error.release_value();
 
-                auto done_property = TRY(next_object->get(names.done));
+            // 3. Let n be 0.
-                if (done_property.to_boolean()) {
+            // 4. Repeat,
-                    iterator_done = true;
+            while (true) {
+                ThrowCompletionOr<Object*> next { nullptr };
+
+                // a. If iteratorRecord.[[Done]] is false, then
+                if (!iterator_record.done) {
+                    // i. Let next be IteratorStep(iteratorRecord).
+                    next = iterator_step(global_object, iterator_record);
+
+                    // ii. If next is an abrupt completion, set iteratorRecord.[[Done]] to true.
+                    // iii. ReturnIfAbrupt(next).
+                    if (next.is_error()) {
+                        iterator_record.done = true;
+                        return next.release_error();
+                    }
+
+                    // iv. If next is false, set iteratorRecord.[[Done]] to true.
+                    if (!next.value())
+                        iterator_record.done = true;
+                }
+
+                // b. If iteratorRecord.[[Done]] is true, then
+                if (iterator_record.done) {
+                    // NOTE: Step i. and ii. are handled below.
                     break;
                 }
 
-                auto next_value = TRY(next_object->get(names.value));
+                // c. Let nextValue be IteratorValue(next).
-                array->indexed_properties().append(next_value);
+                auto next_value = iterator_value(global_object, *next.value());
+
+                // d. If nextValue is an abrupt completion, set iteratorRecord.[[Done]] to true.
+                // e. ReturnIfAbrupt(nextValue).
+                if (next_value.is_error()) {
+                    iterator_record.done = true;
+                    return next_value.release_error();
+                }
+
+                // f. Perform ! CreateDataPropertyOrThrow(A, ! ToString(𝔽(n)), nextValue).
+                array->indexed_properties().append(next_value.value());
+
+                // g. Set n to n + 1.
             }
             value = array;
-
+        }
-        } else if (!iterator_done) {
+        // SingleNameBinding : BindingIdentifier Initializer[opt]
-            auto next_object_or_error = iterator_next(*iterator);
+        // BindingElement : BindingPattern Initializer[opt]
-            if (next_object_or_error.is_throw_completion()) {
+        else {
-                iterator_done = true;
+            // 1. Let v be undefined.
-                return JS::throw_completion(*next_object_or_error.release_error().value());
-            }
-            auto* next_object = next_object_or_error.release_value();
-
-            auto done_property = TRY(next_object->get(names.done));
-            if (done_property.to_boolean()) {
-                iterator_done = true;
-                value = js_undefined();
-            } else {
-                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());
-                }
-                value = value_or_error.release_value();
-            }
-        } else {
             value = js_undefined();
+
+            // 2. If iteratorRecord.[[Done]] is false, then
+            if (!iterator_record.done) {
+                // a. Let next be IteratorStep(iteratorRecord).
+                auto next = iterator_step(global_object, iterator_record);
+
+                // b. If next is an abrupt completion, set iteratorRecord.[[Done]] to true.
+                // c. ReturnIfAbrupt(next).
+                if (next.is_error()) {
+                    iterator_record.done = true;
+                    return next.release_error();
+                }
+
+                // d. If next is false, set iteratorRecord.[[Done]] to true.
+                if (!next.value()) {
+                    iterator_record.done = true;
+                }
+                // e. Else,
+                else {
+                    // i. Set v to IteratorValue(next).
+                    auto value_or_error = iterator_value(global_object, *next.value());
+
+                    // ii. If v is an abrupt completion, set iteratorRecord.[[Done]] to true.
+                    // iii. ReturnIfAbrupt(v).
+                    if (value_or_error.is_throw_completion()) {
+                        iterator_record.done = true;
+                        return value_or_error.release_error();
+                    }
+                    value = value_or_error.release_value();
+                }
+            }
+
+            // NOTE: Step 3. and 4. are handled below.
         }
 
         if (value.is_undefined() && entry.initializer) {

Userland/Libraries/LibJS/Runtime/VM.h

@@ -1,6 +1,6 @@
 /*
  * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
- * Copyright (c) 2020-2021, Linus Groh <linusg@serenityos.org>
+ * Copyright (c) 2020-2022, Linus Groh <linusg@serenityos.org>
  * Copyright (c) 2021, David Tuin <davidot@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -21,6 +21,7 @@
 #include <LibJS/Runtime/ErrorTypes.h>
 #include <LibJS/Runtime/Exception.h>
 #include <LibJS/Runtime/ExecutionContext.h>
+#include <LibJS/Runtime/Iterator.h>
 #include <LibJS/Runtime/MarkedValueList.h>
 #include <LibJS/Runtime/Promise.h>
 #include <LibJS/Runtime/Value.h>
@@ -244,7 +245,7 @@ private:
     [[nodiscard]] ThrowCompletionOr<Value> call_internal(FunctionObject&, Value this_value, Optional<MarkedValueList> arguments);
 
     ThrowCompletionOr<void> property_binding_initialization(BindingPattern const& binding, Value value, Environment* environment, GlobalObject& global_object);
-    ThrowCompletionOr<void> iterator_binding_initialization(BindingPattern const& binding, Object* iterator, bool& iterator_done, Environment* environment, GlobalObject& global_object);
+    ThrowCompletionOr<void> iterator_binding_initialization(BindingPattern const& binding, Iterator& iterator_record, Environment* environment, GlobalObject& global_object);
 
     Exception* m_exception { nullptr };