From 2e28f292d35a2928b77117c692e052368f9c68e0 Mon Sep 17 00:00:00 2001
From: Bagaev Dmitry <bvdmitri@gmail.com>
Date: Tue, 15 Sep 2026 14:31:19 +0200
Subject: [PATCH] fix: throw a descriptive error when iterating a sparse
 `ResizableArray`

`Base.iterate` indexed by the reported (max-extent) `size` without an
`isassigned` guard, so iterating a ragged or sparsely filled array raised a
bare `BoundsError` pointing at an internal vector, with no hint about the real
problem or the supported alternative.

`iterate` now checks `isassigned` and raises an error naming the unassigned
index and pointing at `GraphPPL.vec`, which already guards with `isassigned`
and is the supported way to traverse ragged arrays.

Note it deliberately does *not* skip unassigned slots. `ResizableArray <:
AbstractArray` and there is no `length`/`IteratorSize` override, so
`length == prod(size)`; skipping would break the iteration protocol and leave
`collect`/`map` results holding uninitialized memory. Failing loudly is the
safer contract for a package that must build factor graphs deterministically.

Also guards the empty case in the 0-argument method, which previously
destructured `iterate(CartesianIndices(...))` unconditionally.

Closes #308.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/resizable_array.jl        | 25 ++++++++++++++++++++++---
 test/resizable_array_tests.jl | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/src/resizable_array.jl b/src/resizable_array.jl
index 76e75176..7866bdbc 100644
--- a/src/resizable_array.jl
+++ b/src/resizable_array.jl
@@ -172,12 +172,31 @@ function vec(array::ResizableArray{T, V, N}) where {T, V, N}
     return result
 end
 
+# `ResizableArray` is an `AbstractArray`, so `length` (and thus `collect`, `map`, broadcasting, ...)
+# reports the maximum extent, not the number of assigned elements. Silently skipping unassigned slots
+# would therefore break the iteration protocol and leave uninitialized memory in the result, so
+# iteration over a ragged or sparsely filled array fails loudly instead. Use `vec` to traverse
+# only the assigned elements.
+function __resizable_array_checked_getindex(array::ResizableArray, index::CartesianIndex)
+    if !(isassigned(array, index.I...)::Bool)
+        error(
+            lazy"Cannot iterate over `ResizableArray` at index $(index.I) because this slot is unassigned. Ragged or sparsely filled arrays cannot be iterated densely, use `GraphPPL.vec(array)` to iterate over the assigned elements only."
+        )
+    end
+    return array[index.I...]
+end
+
 function Base.iterate(array::ResizableArray)
     # We want to emulate the same iteration protocol as for the `Array` structure 
     # which iterates over the last dimension first
     indx = CartesianIndices(size(array))
-    pindex, pstate = iterate(indx)
-    return (array[pindex.I...], isnothing(pstate) ? nothing : (indx, pstate))
+    piterate = iterate(indx)
+    # An empty array has nothing to iterate over
+    if isnothing(piterate)
+        return nothing
+    end
+    pindex, pstate = piterate
+    return (__resizable_array_checked_getindex(array, pindex), isnothing(pstate) ? nothing : (indx, pstate))
 end
 
 function Base.iterate(array::ResizableArray, state)
@@ -190,7 +209,7 @@ function Base.iterate(array::ResizableArray, state)
         return nothing
     end
     nindex, nstate = niterate
-    return (array[nindex.I...], isnothing(nstate) ? nothing : (indx, nstate))
+    return (__resizable_array_checked_getindex(array, nindex), isnothing(nstate) ? nothing : (indx, nstate))
 end
 
 __length(array::ResizableArray{T, V, N}) where {T, V, N} = __recursive_length(Val(N), array.data)
diff --git a/test/resizable_array_tests.jl b/test/resizable_array_tests.jl
index 7338dee6..3ee249b3 100644
--- a/test/resizable_array_tests.jl
+++ b/test/resizable_array_tests.jl
@@ -371,4 +371,36 @@ end
             @test elem[] === 1
         end
     end
+
+    # Iterating a ragged / sparsely filled array must fail loudly and point at `vec`.
+    # It must never silently skip the unassigned slots: `ResizableArray <: AbstractArray` reports
+    # `length == prod(size)`, so skipping would leave uninitialized memory in `collect`/`map` results.
+    @testset "sparse ResizableArray should throw a descriptive error on iteration" begin
+        a = ResizableArray(Float64, Val(2))
+        a[1, 1] = 1.0
+        a[1, 2] = 2.0
+        a[2, 1] = 3.0
+
+        @test size(a) === (2, 2)
+        @test !isassigned(a, 2, 2)
+
+        for f in (collect, x -> map(identity, x), x -> [e for e in x], x -> (for e in x
+        end))
+            @test_throws "this slot is unassigned" f(a)
+            @test_throws "GraphPPL.vec" f(a)
+        end
+
+        # `vec` remains the supported way to traverse the assigned elements
+        @test GraphPPL.vec(a) == [1.0, 3.0, 2.0]
+    end
+
+    # An empty array has nothing to iterate over, it should not throw
+    @testset "empty ResizableArray" begin
+        for N in (1, 2, 3)
+            empty_array = ResizableArray(Float64, Val(N))
+            @test isempty(collect(empty_array))
+            @test length(collect(empty_array)) === 0
+            @test isempty(GraphPPL.vec(empty_array))
+        end
+    end
 end