LibJS: Revert ArrayIterator and RegExpStringIterator to manual iterators

This is a normative change in the ECMA-262 spec. See:
de62e8d

This did not actually seem to affect our implementation as we were not
using generators here to begin with. So this patch is basically just
adding spec comments.

Libraries/LibJS/Runtime/ArrayIterator.cpp

@@ -11,8 +11,14 @@ namespace JS {
 
 GC_DEFINE_ALLOCATOR(ArrayIterator);
 
+// 23.1.5.1 CreateArrayIterator ( array, kind ), https://tc39.es/ecma262/#sec-createarrayiterator
 GC::Ref<ArrayIterator> ArrayIterator::create(Realm& realm, Value array, Object::PropertyKind iteration_kind)
 {
+    // 1. Let iterator be OrdinaryObjectCreate(%ArrayIteratorPrototype%, « [[IteratedArrayLike]], [[ArrayLikeNextIndex]], [[ArrayLikeIterationKind]] »).
+    // 2. Set iterator.[[IteratedArrayLike]] to array.
+    // 3. Set iterator.[[ArrayLikeNextIndex]] to 0.
+    // 4. Set iterator.[[ArrayLikeIterationKind]] to kind.
+    // 5. Return iterator.
     return realm.create<ArrayIterator>(array, iteration_kind, realm.intrinsics().array_iterator_prototype());
 }
 

Libraries/LibJS/Runtime/ArrayIterator.h

@@ -20,12 +20,14 @@ public:
     virtual ~ArrayIterator() override = default;
 
     Value array() const { return m_array; }
+    void set_array(Value array) { m_array = array; }
+
     Object::PropertyKind iteration_kind() const { return m_iteration_kind; }
+
     size_t index() const { return m_index; }
+    void set_index(size_t index) { m_index = index; }
 
 private:
-    friend class ArrayIteratorPrototype;
-
     ArrayIterator(Value array, Object::PropertyKind iteration_kind, Object& prototype);
 
     virtual bool is_array_iterator() const override { return true; }

Libraries/LibJS/Runtime/ArrayIteratorPrototype.cpp

@@ -33,93 +33,104 @@ void ArrayIteratorPrototype::initialize(Realm& realm)
 }
 
 // 23.1.5.2.1 %ArrayIteratorPrototype%.next ( ), https://tc39.es/ecma262/#sec-%arrayiteratorprototype%.next
-// FIXME: This seems to be CreateArrayIterator (https://tc39.es/ecma262/#sec-createarrayiterator) instead of %ArrayIteratorPrototype%.next.
 JS_DEFINE_NATIVE_FUNCTION(ArrayIteratorPrototype::next)
 {
     auto& realm = *vm.current_realm();
 
+    // 1. Let O be the this value.
+    // 2. If O is not an Object, throw a TypeError exception.
+    // 3. If O does not have all of the internal slots of an Array Iterator Instance (23.1.5.3), throw a TypeError exception.
     auto iterator = TRY(typed_this_value(vm));
+
+    // 4. Let array be O.[[IteratedArrayLike]].
     auto target_array = iterator->array();
+
+    // 5. If array is undefined, return CreateIteratorResultObject(undefined, true).
     if (target_array.is_undefined())
         return create_iterator_result_object(vm, js_undefined(), true);
+
     VERIFY(target_array.is_object());
     auto& array = target_array.as_object();
 
+    // 6. Let index be O.[[ArrayLikeNextIndex]].
     auto index = iterator->index();
-    auto iteration_kind = iterator->iteration_kind();
 
-    size_t length;
+    // 7. Let kind be O.[[ArrayLikeIterationKind]].
+    auto kind = iterator->iteration_kind();
 
-    // i. If array has a [[TypedArrayName]] internal slot, then
+    size_t length = 0;
+
+    // 8. If array has a [[TypedArrayName]] internal slot, then
     if (array.is_typed_array()) {
         auto& typed_array = static_cast<TypedArrayBase&>(array);
 
-        // 1. Let taRecord be MakeTypedArrayWithBufferWitnessRecord(array, seq-cst).
+        // a. Let taRecord be MakeTypedArrayWithBufferWitnessRecord(array, SEQ-CST).
         auto typed_array_record = make_typed_array_with_buffer_witness_record(typed_array, ArrayBuffer::SeqCst);
 
-        // 2. If IsTypedArrayOutOfBounds(taRecord) is true, throw a TypeError exception.
+        // b. If IsTypedArrayOutOfBounds(taRecord) is true, throw a TypeError exception.
         if (is_typed_array_out_of_bounds(typed_array_record))
             return vm.throw_completion<TypeError>(ErrorType::BufferOutOfBounds, "TypedArray"sv);
 
-        // 3. Let len be TypedArrayLength(taRecord).
+        // c. Let len be TypedArrayLength(taRecord).
         length = typed_array_length(typed_array_record);
     }
-    // ii. Else,
+    // 9. Else,
     else {
-        // 1. Let len be ? LengthOfArrayLike(array).
+        // a. Let len be ? LengthOfArrayLike(array).
         length = TRY(length_of_array_like(vm, array));
     }
 
-    // iii. If index ≥ len, return NormalCompletion(undefined).
+    // 10. If index ≥ len, then
     if (index >= length) {
-        iterator->m_array = js_undefined();
+        // a. Set O.[[IteratedArrayLike]] to undefined.
+        iterator->set_array(js_undefined());
+
+        // b. Return CreateIteratorResultObject(undefined, true).
         return create_iterator_result_object(vm, js_undefined(), true);
     }
 
-    // iv. Let indexNumber be 𝔽(index).
+    // 11. Set O.[[ArrayLikeNextIndex]] to index + 1.
+    iterator->set_index(index + 1);
+
+    // 12. Let indexNumber be 𝔽(index).
 
     Value result;
 
-    // v. If kind is key, then
-    if (iteration_kind == Object::PropertyKind::Key) {
-        // 1. Let result be indexNumber.
-        result = Value(static_cast<i32>(index));
+    // 13. If kind is KEY, then
+    if (kind == PropertyKind::Key) {
+        // a. Let result be indexNumber.
+        result = Value { static_cast<i32>(index) };
     }
-    // vi. Else,
+    // 14. Else,
     else {
-        // 1. Let elementKey be ! ToString(indexNumber).
-        // 2. Let elementValue be ? Get(array, elementKey).
+        // a. Let elementKey be ! ToString(indexNumber).
+        // b. Let elementValue be ? Get(array, elementKey).
         auto element_value = TRY([&]() -> ThrowCompletionOr<Value> {
             // OPTIMIZATION: For objects that don't interfere with indexed property access, we try looking directly at storage.
             if (!array.may_interfere_with_indexed_property_access() && array.indexed_properties().has_index(index)) {
-                auto value = array.indexed_properties().get(index)->value;
-                if (!value.is_accessor()) {
+                if (auto value = array.indexed_properties().get(index)->value; !value.is_accessor())
                     return value;
-                }
             }
 
             return array.get(index);
         }());
 
-        // 3. If kind is value, then
-        if (iteration_kind == Object::PropertyKind::Value) {
-            // a. Let result be elementValue.
+        // c. If kind is VALUE, then
+        if (kind == PropertyKind::Value) {
+            // i. Let result be elementValue.
             result = element_value;
         }
-        // 4. Else,
+        // d. Else,
         else {
-            // a. Assert: kind is key+value.
-            VERIFY(iteration_kind == Object::PropertyKind::KeyAndValue);
+            // i. Assert: kind is KEY+VALUE.
+            VERIFY(kind == PropertyKind::KeyAndValue);
 
-            // b. Let result be CreateArrayFromList(« indexNumber, elementValue »).
+            // ii. Let result be CreateArrayFromList(« indexNumber, elementValue »).
             result = Array::create_from(realm, { Value(static_cast<i32>(index)), element_value });
         }
     }
 
-    // viii. Set index to index + 1.
-    ++iterator->m_index;
-
-    // vii. Perform ? GeneratorYield(CreateIterResultObject(result, false)).
+    // 15. Return CreateIteratorResultObject(result, false).
     return create_iterator_result_object(vm, result, false);
 }
 

Libraries/LibJS/Runtime/RegExpStringIterator.cpp

@@ -14,6 +14,13 @@ GC_DEFINE_ALLOCATOR(RegExpStringIterator);
 // 22.2.9.1 CreateRegExpStringIterator ( R, S, global, fullUnicode ), https://tc39.es/ecma262/#sec-createregexpstringiterator
 GC::Ref<RegExpStringIterator> RegExpStringIterator::create(Realm& realm, Object& regexp_object, Utf16String string, bool global, bool unicode)
 {
+    // 1. Let iterator be OrdinaryObjectCreate(%RegExpStringIteratorPrototype%, « [[IteratingRegExp]], [[IteratedString]], [[Global]], [[Unicode]], [[Done]] »).
+    // 2. Set iterator.[[IteratingRegExp]] to R.
+    // 3. Set iterator.[[IteratedString]] to S.
+    // 4. Set iterator.[[Global]] to global.
+    // 5. Set iterator.[[Unicode]] to fullUnicode.
+    // 6. Set iterator.[[Done]] to false.
+    // 7. Return iterator.
     return realm.create<RegExpStringIterator>(realm.intrinsics().regexp_string_iterator_prototype(), regexp_object, move(string), global, unicode);
 }
 

Libraries/LibJS/Runtime/RegExpStringIterator.h

@@ -22,7 +22,7 @@ public:
     virtual ~RegExpStringIterator() override = default;
 
     Object& regexp_object() { return m_regexp_object; }
-    Utf16String string() const { return m_string; }
+    Utf16String const& string() const { return m_string; }
     bool global() const { return m_global; }
     bool unicode() const { return m_unicode; }
 

Libraries/LibJS/Runtime/RegExpStringIteratorPrototype.cpp

@@ -34,35 +34,66 @@ void RegExpStringIteratorPrototype::initialize(Realm& realm)
 // 22.2.9.2.1 %RegExpStringIteratorPrototype%.next ( ), https://tc39.es/ecma262/#sec-%regexpstringiteratorprototype%.next
 JS_DEFINE_NATIVE_FUNCTION(RegExpStringIteratorPrototype::next)
 {
-    // For details, see the 'closure' of: https://tc39.es/ecma262/#sec-createregexpstringiterator
+    // 1. Let O be the this value.
+    // 2. If O is not an Object, throw a TypeError exception.
+    // 3. If O does not have all of the internal slots of a RegExp String Iterator Object Instance (see 22.2.9.3), throw a TypeError exception.
     auto iterator = TRY(typed_this_value(vm));
-    if (iterator->done())
-        return create_iterator_result_object(vm, js_undefined(), true);
 
-    auto match = TRY(regexp_exec(vm, iterator->regexp_object(), iterator->string()));
-
-    if (match.is_null()) {
-        iterator->set_done();
+    // 4. If O.[[Done]] is true, then
+    if (iterator->done()) {
+        // a. Return CreateIteratorResultObject(undefined, true).
         return create_iterator_result_object(vm, js_undefined(), true);
     }
 
-    if (!iterator->global()) {
+    // 5. Let R be O.[[IteratingRegExp]].
+    auto& regexp = iterator->regexp_object();
+
+    // 6. Let S be O.[[IteratedString]].
+    auto const& string = iterator->string();
+
+    // 7. Let global be O.[[Global]].
+    auto global = iterator->global();
+
+    // 8. Let fullUnicode be O.[[Unicode]].
+    auto full_unicode = iterator->unicode();
+
+    // 9. Let match be ? RegExpExec(R, S).
+    auto match = TRY(regexp_exec(vm, regexp, string));
+
+    // 10. If match is null, then
+    if (match.is_null()) {
+        // a. Set O.[[Done]] to true.
         iterator->set_done();
+
+        // b. Return CreateIteratorResultObject(undefined, true).
+        return create_iterator_result_object(vm, js_undefined(), true);
+    }
+
+    // 11. If global is false, then
+    if (!global) {
+        // a. Set O.[[Done]] to true.
+        iterator->set_done();
+
+        // b. Return CreateIteratorResultObject(match, false).
         return create_iterator_result_object(vm, match, false);
     }
 
-    auto match_object = TRY(match.to_object(vm));
-    auto match_string_value = TRY(match_object->get(0));
-    auto match_string = TRY(match_string_value.to_string(vm));
+    // 12. Let matchStr be ? ToString(? Get(match, "0")).
+    auto match_string = TRY(TRY(match.get(vm, 0)).to_utf16_string(vm));
+
+    // 13. If matchStr is the empty String, then
     if (match_string.is_empty()) {
-        auto last_index_value = TRY(iterator->regexp_object().get(vm.names.lastIndex));
-        auto last_index = TRY(last_index_value.to_length(vm));
+        // a. Let thisIndex be ℝ(? ToLength(? Get(R, "lastIndex"))).
+        auto this_index = TRY(TRY(regexp.get(vm.names.lastIndex)).to_length(vm));
 
-        last_index = advance_string_index(iterator->string().view(), last_index, iterator->unicode());
+        // b. Let nextIndex be AdvanceStringIndex(S, thisIndex, fullUnicode).
+        auto next_index = advance_string_index(string.view(), this_index, full_unicode);
 
-        TRY(iterator->regexp_object().set(vm.names.lastIndex, Value(last_index), Object::ShouldThrowExceptions::Yes));
+        // c. Perform ? Set(R, "lastIndex", 𝔽(nextIndex), true).
+        TRY(regexp.set(vm.names.lastIndex, Value { next_index }, Object::ShouldThrowExceptions::Yes));
     }
 
+    // 14. Return CreateIteratorResultObject(match, false).
     return create_iterator_result_object(vm, match, false);
 }