diff --git a/builtin/array.mbt b/builtin/array.mbt index cb986565b..c0b8d2004 100644 --- a/builtin/array.mbt +++ b/builtin/array.mbt @@ -415,10 +415,13 @@ pub impl[T] Add for Array[T] with fn add(self, other) { /// inspect(sum, content="6") /// } /// ``` -/// This method uses the array iterator. Structural mutations during traversal -/// are unsupported: appended elements are not visited, and shrinking the array -/// with operations such as `remove`, `truncate`, `clear`, or `drain` may cause -/// later iterator steps to fail. +/// This method traverses the array via `for .. in`, whose current compiler +/// lowering fixes the traversal bounds when iteration starts. Structural +/// mutations during traversal are therefore unsupported here (unlike +/// `Array::iter`, which has live semantics): appended elements are not +/// visited, and shrinking the array with operations such as `remove`, +/// `truncate`, `clear`, or `drain` may cause later steps to read invalid +/// slots. Use `retain()` or `retain_map()` to delete elements by predicate. #locals(f) pub fn[T] Array::each(self : Array[T], f : (T) -> Unit raise?) -> Unit raise? { for v in self { @@ -1306,13 +1309,12 @@ pub fn[T] Array::repeat(self : Array[T], times : Int) -> Array[T] { ///| /// Fold out values from an array according to certain rules. -/// This method traverses the array through `self.iter()`, so the traversal -/// bounds are fixed when folding starts. -/// -/// Structural mutations to `self` inside `f` are unsupported. Appended -/// elements are not visited, and shrinking the array with operations such as -/// `remove`, `truncate`, `clear`, or `drain` may cause later fold steps to -/// fail. +/// This method traverses the array via `for .. in`, whose current compiler +/// lowering fixes the traversal bounds when folding starts. Structural +/// mutations to `self` inside `f` are therefore unsupported here (unlike +/// `Array::iter`, which has live semantics): appended elements are not +/// visited, and shrinking the array may cause later steps to read invalid +/// slots. /// /// Example: /// @@ -1730,14 +1732,25 @@ pub fn[T] Array::split( /// * `array` : The array to create an iterator from. /// /// Returns an iterator that yields each element of the array in order. -/// This iterator is created from `self[:]`, so the traversal bounds are fixed -/// when iteration starts. /// -/// Structural mutations after the iterator is created are unsupported. -/// Appended elements are not visited, and shrinking the array with operations -/// such as `remove`, `truncate`, `clear`, or `drain` may cause later iterator -/// steps to fail. The same caveat applies to `rev_iter()`, `iter2()`, and -/// helpers built on top of them such as `each()`, `eachi()`, and `fold()`. +/// The iterator reads the array's length and buffer afresh at every step +/// (live semantics), so it stays type- and memory-safe even if the array is +/// structurally mutated during iteration: +/// +/// * elements appended during iteration are visited (so appending on every +/// step never terminates); +/// * removing an already-visited or current element shifts its successors one +/// slot left, so the element moving into the current position is skipped; +/// * once the array shrinks to at most the current position, iteration stops. +/// +/// Prefer not to mutate the array while iterating: use `retain()` or +/// `retain_map()` to delete elements by predicate, or iterate a view +/// (`xs[:].iter()`) to traverse a snapshot whose bounds are fixed up front. +/// `rev_iter()` and `iter2()` have the same live semantics. Note that +/// `for x in xs` and helpers written with it (`each()`, `eachi()`, `fold()`) +/// currently use the compiler's specialized lowering, which still fixes the +/// traversal bounds up front; they do not get live semantics until the +/// compiler lowering is updated to match. /// /// Example: /// @@ -1751,7 +1764,16 @@ pub fn[T] Array::split( /// ``` #alias(iterator, deprecated) pub fn[T] Array::iter(self : Array[T]) -> Iter[T] { - self[:].iter() + let mut i = 0 + Iter::new( + fn() { + guard i < self.length() else { None } + let elem = self.unsafe_get(i) + i += 1 + Some(elem) + }, + size_hint=self.length(), + ) } ///| @@ -1765,6 +1787,13 @@ pub fn[T] Array::iter(self : Array[T]) -> Iter[T] { /// Returns an iterator that yields each element of the array, starting from the /// last element and moving towards the first. /// +/// Like `iter()`, the iterator re-reads the array's length and buffer at every +/// step (live semantics), so it stays type- and memory-safe under structural +/// mutation: if the array shrinks below the cursor, the cursor is clamped to +/// the new length; removing a not-yet-visited element shifts its successors +/// left, causing an already-visited element to be revisited. Prefer not to +/// mutate the array while iterating. +/// /// Example: /// /// ```mbt check @@ -1777,7 +1806,19 @@ pub fn[T] Array::iter(self : Array[T]) -> Iter[T] { /// ``` #alias(rev_iterator, deprecated) pub fn[T] Array::rev_iter(self : Array[T]) -> Iter[T] { - self[:].rev_iter() + let mut i = self.length() + Iter::new( + fn() { + let len = self.length() + if i > len { + i = len + } + guard i > 0 else { None } + i -= 1 + Some(self.unsafe_get(i)) + }, + size_hint=self.length(), + ) } ///| @@ -1791,6 +1832,10 @@ pub fn[T] Array::rev_iter(self : Array[T]) -> Iter[T] { /// Returns an iterator that yields tuples of index and value pairs, where /// indices start from 0. /// +/// Like `iter()`, the iterator re-reads the array's length and buffer at every +/// step (live semantics), so it stays type- and memory-safe under structural +/// mutation. Prefer not to mutate the array while iterating. +/// /// Example: /// /// ```mbt check @@ -1803,7 +1848,16 @@ pub fn[T] Array::rev_iter(self : Array[T]) -> Iter[T] { /// ``` #alias(iterator2, deprecated) pub fn[A] Array::iter2(self : Array[A]) -> Iter2[Int, A] { - self[:].iter2() + let mut i = 0 + Iter2::new( + fn() { + guard i < self.length() else { None } + let result = Some((i, self.unsafe_get(i))) + i += 1 + result + }, + size_hint=self.length(), + ) } ///| diff --git a/builtin/array_test.mbt b/builtin/array_test.mbt index 0a687571b..9fd58f060 100644 --- a/builtin/array_test.mbt +++ b/builtin/array_test.mbt @@ -1599,3 +1599,82 @@ test "timsort_early_return_for_small_arrays" { // With the bug, we get at least 45 + 9 = 54 comparisons. inspect(count.val, content="45") } + +///| +test "Array::iter/live_clear_during_iteration" { + let xs = [1, 2, 3] + let visited = [] + let iter = xs.iter() + while iter.next() is Some(x) { + visited.push(x) + xs.clear() + } + debug_inspect(visited, content="[1]") + debug_inspect(xs, content="[]") +} + +///| +test "Array::iter/live_remove_skips_successor" { + let xs = [1, 2, 3, 4] + let visited = [] + let iter = xs.iter() + while iter.next() is Some(x) { + visited.push(x) + if x == 2 { + ignore(xs.remove(1)) + } + } + // removing the current element shifts 3 into the visited slot, so it is + // skipped; iteration continues safely with the live length + debug_inspect(visited, content="[1, 2, 4]") + debug_inspect(xs, content="[1, 3, 4]") +} + +///| +test "Array::iter/live_push_visits_appended" { + let xs = Array::new(capacity=2) + xs.push(1) + xs.push(2) + let visited = [] + let iter = xs.iter() + while iter.next() is Some(x) { + if x == 1 { + // forces a reallocation: the iterator must re-read the live buffer + xs.push(3) + } + visited.push(x) + } + debug_inspect(visited, content="[1, 2, 3]") +} + +///| +test "Array::rev_iter/live_shrink_clamps_cursor" { + let xs = [1, 2, 3, 4] + let visited = [] + xs + .rev_iter() + .each(x => { + visited.push(x) + ignore(xs.pop()) + ignore(xs.pop()) + }) + // after yielding 4 the array shrinks to [1, 2]; the cursor clamps to the + // new length and yields 2, then the array empties and iteration stops + debug_inspect(visited, content="[4, 2]") + debug_inspect(xs, content="[]") +} + +///| +test "Array::iter2/live_remove_during_iteration" { + let xs = [10, 20, 30, 40] + let visited = [] + xs + .iter2() + .each((i, x) => { + visited.push((i, x)) + if x == 20 { + ignore(xs.remove(1)) + } + }) + debug_inspect(visited, content="[(0, 10), (1, 20), (2, 40)]") +}