[JSC] Iterator Helpers methods should not iterate an array

sosukesuzuki · Alexey Shvayka · commit ea7b8287cc3f · 2024-09-19T10:55:53.000-07:00
https://bugs.webkit.org/show_bug.cgi?id=279876 Reviewed by Alexey Shvayka. We implemented a function called `forEachInIteratorProtocol` to implement `Iterator.prototype.toArray`[1]. As the name suggests, this function iterates over objects that conform to the iterator protocol, but it also iterates over fast arrays. The methods of Iterator Helpers use the `GetIteratorDirect` abstract operation[2] to directly get the object's `next` property. Unlike the `GetIterator` abstract operation[3], it does not get the `@@iterator`. This patch changes the `forEachInIteratorProtocol` function to stop iterating over arrays. It also removes the fast path for fast arrays in `Iterator.prototype.toArray`[4]. [1]: https://commits.webkit.org/283381@main [2]: https://tc39.es/proposal-iterator-helpers/#sec-getiteratordirect [3]: https://tc39.es/ecma262/#sec-getiterator [4]: https://commits.webkit.org/283381@main * JSTests/microbenchmarks/iterator-prototype-toArray-for-array.js: Removed. * JSTests/stress/iterator-prototype-forEach.js: (sameArray): (const.validIter): Deleted. (const.invalidCallback.of.invalidCallbacks.shouldThrow): Deleted. * JSTests/stress/iterator-prototype-toArray-empty.js: Removed. * JSTests/stress/iterator-prototype-toArray.js: (sameArray): * Source/JavaScriptCore/runtime/IteratorOperations.h: (JSC::forEachInIteratorProtocol): * Source/JavaScriptCore/runtime/JSIteratorPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): Canonical link: https://commits.webkit.org/283933@main
diff --git a/JSTests/microbenchmarks/iterator-prototype-toArray-for-array.js b/JSTests/microbenchmarks/iterator-prototype-toArray-for-array.js
diff --git a/JSTests/stress/iterator-prototype-forEach.js b/JSTests/stress/iterator-prototype-forEach.js
@@ -101,9 +101,9 @@ function shouldThrow(fn, error, message) {
 
 {
     const arr = [1, 2, 3, 4, 5];
-    const result = [];
-    Iterator.prototype.forEach.call(arr, (item, i) => { result.push([item, i]); });
-    sameArray(result, [[1, 0], [2, 1], [3, 2], [4, 3], [5, 4]]);
+    shouldThrow(function () {
+        Iterator.prototype.forEach.call(arr, (item, i) => {});
+    }, TypeError, "Type error")
 }
 
 {
diff --git a/JSTests/stress/iterator-prototype-toArray-empty.js b/JSTests/stress/iterator-prototype-toArray-empty.js
diff --git a/JSTests/stress/iterator-prototype-toArray.js b/JSTests/stress/iterator-prototype-toArray.js
@@ -95,9 +95,9 @@ function shouldThrow(fn, error, message) {
 
 {
     const arr1 = [1, 2, 3, 4, 5];
-    const arr2 = Iterator.prototype.toArray.call(arr1);
-    sameValue(arr1 === arr2, false);
-    sameArray(arr1, arr2);
+    shouldThrow(function () {
+        Iterator.prototype.toArray.call(arr1);
+    }, TypeError, "Type error");
 }
 
 {
diff --git a/Source/JavaScriptCore/runtime/IteratorOperations.h b/Source/JavaScriptCore/runtime/IteratorOperations.h
@@ -173,13 +173,6 @@ void forEachInIteratorProtocol(JSGlobalObject* globalObject, JSValue iterable, c
     auto& vm = getVM(globalObject);
     auto scope = DECLARE_THROW_SCOPE(vm);
 
-    if (getIterationMode(vm, globalObject, iterable) == IterationMode::FastArray) {
-        auto* array = jsCast<JSArray*>(iterable);
-        forEachInFastArray(globalObject, iterable, array, callback);
-        RETURN_IF_EXCEPTION(scope, void());
-        return;
-    }
-
     IterationRecord iterationRecord = iteratorDirect(globalObject, iterable);
     RETURN_IF_EXCEPTION(scope, void());
     scope.release();
diff --git a/Source/JavaScriptCore/runtime/JSIteratorPrototype.cpp b/Source/JavaScriptCore/runtime/JSIteratorPrototype.cpp
@@ -152,13 +152,6 @@ JSC_DEFINE_HOST_FUNCTION(iteratorProtoFuncToArray, (JSGlobalObject* globalObject
     if (!thisValue.isObject())
         return throwVMTypeError(globalObject, scope, "Iterator.prototype.toArray requires that |this| be an Object."_s);
 
-    if (getIterationMode(vm, globalObject, thisValue) == IterationMode::FastArray) {
-        JSArray* array = tryCloneArrayFromFast(globalObject, thisValue);
-        RETURN_IF_EXCEPTION(scope, { });
-        if (array)
-            return JSValue::encode(array);
-    }
-
     MarkedArgumentBuffer value;
     forEachInIteratorProtocol(globalObject, thisValue, [&value, &scope](VM&, JSGlobalObject* globalObject, JSValue nextItem) {
         value.append(nextItem);

Original file line number	Diff line number	Diff line change
`@@ -101,9 +101,9 @@ function shouldThrow(fn, error, message) {`
`101`	`101`
`102`	`102`	`{`
`103`	`103`	`const arr = [1, 2, 3, 4, 5];`
`104`		`- const result = [];`
`105`		`- Iterator.prototype.forEach.call(arr, (item, i) => { result.push([item, i]); });`
`106`		`- sameArray(result, [[1, 0], [2, 1], [3, 2], [4, 3], [5, 4]]);`
	`104`	`+ shouldThrow(function () {`
	`105`	`+ Iterator.prototype.forEach.call(arr, (item, i) => {});`
	`106`	`+ }, TypeError, "Type error")`
`107`	`107`	`}`
`108`	`108`
`109`	`109`	`{`
Original file line number	Diff line number	Diff line change
`@@ -95,9 +95,9 @@ function shouldThrow(fn, error, message) {`
`95`	`95`
`96`	`96`	`{`
`97`	`97`	`const arr1 = [1, 2, 3, 4, 5];`
`98`		`- const arr2 = Iterator.prototype.toArray.call(arr1);`
`99`		`- sameValue(arr1 === arr2, false);`
`100`		`- sameArray(arr1, arr2);`
	`98`	`+ shouldThrow(function () {`
	`99`	`+ Iterator.prototype.toArray.call(arr1);`
	`100`	`+ }, TypeError, "Type error");`
`101`	`101`	`}`
`102`	`102`
`103`	`103`	`{`