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