aboutsummaryrefslogtreecommitdiffstats
path: root/src/qml/jsruntime/qv4setobject.cpp
diff options
context:
space:
mode:
authorLuca Di Sera <luca.disera@qt.io>2024-08-27 17:05:39 +0200
committerLuca Di Sera <luca.disera@qt.io>2024-08-30 00:28:35 +0200
commit3244c592935448c9b6e907fe6b67498f1649136c (patch)
tree13076da4764fc5ef617de60ab3355d12b041990e /src/qml/jsruntime/qv4setobject.cpp
parentfc740db1c6f5bdfb2aa53ab392d30fe00dd52aba (diff)
QJSEngine: Do not skip values in Map/Set.prototype.forEach
The backing storage implementation for Maps/Sets, `ESTable`, currently uses a pair of arrays for its storage of keys and values, respectively. Additional elements that should be added are appended to the arrays. When an element is removed, all elements past it are shifted to the left, to avoid having empty spaces. The arrays naturally preserve insertion order, which is required to be the iteration order for `forEach` based on the spec. `Map/Set.prototype.forEach` implementations make use of this fact by iterating per-index so that the iteration follows the required order. As per the spec, during the execution of a `Map/Set.prototype.forEach` call, a call to `callbackFn` might modify the iterated upon collection itself. Depending on the specific changes that are performed, this might break the iteration cycle in the index-based iteration that the `forEach` implementation performs. For example, given: ``` let set = new Set([1,2,3]); set.forEach((v) => { console.log(v) set.delete(v) }) ``` The index based implementation would perform the following iterations: - Set = [1, 2, 3]; index = 0; - visit 1 - 1 is deleted from the Set - ... - Set = [2, 3]; index = 1; - visit 3 ... Skipping the required visit of 2 due to the index not being re-adjusted between iterations. To avoid the issue, the way that `forEach` implementations iterate over an `ESTable` was slightly modified. `ESTable` now exposes a simple structure that can be used to observe changes in position in relation to a an indexed entry. An instance of the structure can be registered with the table to observe the internal changes that the table performs, allowing the re-adjustment of an index during an index-based iteration. A small test was added to ensure that the case reported in attached bug report is solved. `tst_ecmascripttests` was modified to enable some `forEach` related tests that were previously failing and will now work due to the iteration changes. Fixes: QTBUG-123377 Pick-to: 6.8 Change-Id: I8fc72b747eb295f009c2e2c2933f05b26f1ce717 Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
Diffstat (limited to 'src/qml/jsruntime/qv4setobject.cpp')
-rw-r--r--src/qml/jsruntime/qv4setobject.cpp12
1 files changed, 10 insertions, 2 deletions
diff --git a/src/qml/jsruntime/qv4setobject.cpp b/src/qml/jsruntime/qv4setobject.cpp
index 43cf34d2c3..daf4893ce5 100644
--- a/src/qml/jsruntime/qv4setobject.cpp
+++ b/src/qml/jsruntime/qv4setobject.cpp
@@ -255,15 +255,23 @@ ReturnedValue SetPrototype::method_forEach(const FunctionObject *b, const Value
if (argc > 1)
thisArg = ScopedValue(scope, argv[1]);
+ ESTable::ShiftObserver observer{};
+ that->d()->esTable->observeShifts(observer);
+
Value *arguments = scope.alloc(3);
- for (uint i = 0; i < that->d()->esTable->size(); ++i) {
- that->d()->esTable->iterate(i, &arguments[0], &arguments[1]); // fill in key (0), value (1)
+ while (observer.pivot < that->d()->esTable->size()) {
+ that->d()->esTable->iterate(observer.pivot, &arguments[0], &arguments[1]); // fill in key (0), value (1)
arguments[1] = arguments[0]; // but for set, we want to return the key twice; value is always undefined.
arguments[2] = that;
callbackfn->call(thisArg, arguments, 3);
CHECK_EXCEPTION();
+
+ observer.next();
}
+
+ that->d()->esTable->stopObservingShifts(observer);
+
return Encode::undefined();
}