Skip to content

Commit 9466478

Browse files
mkittilazarusA
authored andcommitted
Add push! implementation for AbstractArray depending only on resize! (JuliaLang#55470)
Fix JuliaLang#55459 In Julia 1.10, `push!` and `append!` would be functional for `AbstractVector` implementations if `resize!` and `setindex!` were defined. As of JuliaLang#51903 by @vtjnash as in Julia 1.11.0-rc2, `append!` now depends on an implementation of `sizehint!` and `push!`. Since `push!` also depends on `append!`, a stack overflow situation can easily be created. To avoid this, this pull request defines the following * Add generic versions of `push!(a::AbstractVector, x)` which do not depend on `append!` * Add default implementation of `sizehint!` that is a no-op The implementation of `push!(a::AbstractVector, x)` is a generic version based on the implementation of `push!(a::Vector, x)` without depending on internals. # Example for SimpleArray Consider the `SimpleArray` example from test/abstractarray.jl: ```julia mutable struct SimpleArray{T} <: AbstractVector{T} els::Vector{T} end Base.size(sa::SimpleArray) = size(sa.els) Base.getindex(sa::SimpleArray, idx...) = getindex(sa.els, idx...) Base.setindex!(sa::SimpleArray, v, idx...) = setindex!(sa.els, v, idx...) Base.resize!(sa::SimpleArray, n) = resize!(sa.els, n) Base.copy(sa::SimpleArray) = SimpleArray(copy(sa.els)) ``` Note that `setindex!` and `resize!` are implemented for `SimpleArray`. ## Julia 1.10.4: push! is functional On Julia 1.10.4, `push!` has a functional implementation for `SimpleArray` ```julia-repl julia> push!(SimpleArray{Int}(zeros(Int,5)), 6) 6-element SimpleArray{Int64}: 0 0 0 0 0 6 ``` ## Julia 1.11.0-rc2 and nightly: push! requires sizehint! and is prone to stack overflow Before this pull request, on Julia 1.11.0-rc2 and nightly, `push!` fails for want of `sizehint!`. ```julia-repl julia> push!(SimpleArray{Int}(zeros(Int,5)), 6) ERROR: MethodError: no method matching sizehint!(::SimpleArray{Int64}, ::Int64) The function `sizehint!` exists, but no method is defined for this combination of argument types. ... ``` After implementing `sizehint!`, `push!` still fails with a stack overflow. ```julia-repl julia> Base.sizehint!(a::SimpleArray, x) = a julia> push!(SimpleArray{Int}(zeros(Int, 5)), 6) Warning: detected a stack overflow; program state may be corrupted, so further execution might be unreliable. ERROR: StackOverflowError: Stacktrace: [1] _append! @ ./array.jl:1344 [inlined] [2] append! @ ./array.jl:1335 [inlined] [3] push!(a::SimpleArray{Int64}, iter::Int64) @ Base ./array.jl:1336 --- the above 3 lines are repeated 79982 more times --- [239950] _append! @ ./array.jl:1344 [inlined] [239951] append! @ ./array.jl:1335 [inlined] ``` This is because the new implementation of `append!` depends on `push!`. ## After this pull request, push! is functional. After this pull request, there is a functional `push!` for `SimpleArray` again as in Julia 1.10.4: ```julia-repl julia> push!(SimpleArray{Int}(zeros(Int, 5), 6) 6-element SimpleArray{Int64}: 0 0 0 0 0 6 ```
1 parent 9aa11f2 commit 9466478

File tree

2 files changed

+41
-0
lines changed

2 files changed

+41
-0
lines changed

base/abstractarray.jl

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3525,13 +3525,45 @@ julia> map(+, [1 2; 3 4], [1,10,100,1000], zeros(3,1)) # iterates until 3rd is
35253525
"""
35263526
map(f, it, iters...) = collect(Generator(f, it, iters...))
35273527

3528+
# Generic versions of push! for AbstractVector
3529+
# These are specialized further for Vector for faster resizing and setindexing
3530+
function push!(a::AbstractVector{T}, item) where T
3531+
# convert first so we don't grow the array if the assignment won't work
3532+
itemT = item isa T ? item : convert(T, item)::T
3533+
new_length = length(a) + 1
3534+
resize!(a, new_length)
3535+
a[new_length] = itemT
3536+
return a
3537+
end
3538+
3539+
# specialize and optimize the single argument case
3540+
function push!(a::AbstractVector{Any}, @nospecialize x)
3541+
new_length = length(a) + 1
3542+
resize!(a, new_length)
3543+
a[new_length] = x
3544+
return a
3545+
end
3546+
function push!(a::AbstractVector{Any}, @nospecialize x...)
3547+
@_terminates_locally_meta
3548+
na = length(a)
3549+
nx = length(x)
3550+
resize!(a, na + nx)
3551+
for i = 1:nx
3552+
a[na+i] = x[i]
3553+
end
3554+
return a
3555+
end
3556+
35283557
# multi-item push!, pushfirst! (built on top of type-specific 1-item version)
35293558
# (note: must not cause a dispatch loop when 1-item case is not defined)
35303559
push!(A, a, b) = push!(push!(A, a), b)
35313560
push!(A, a, b, c...) = push!(push!(A, a, b), c...)
35323561
pushfirst!(A, a, b) = pushfirst!(pushfirst!(A, b), a)
35333562
pushfirst!(A, a, b, c...) = pushfirst!(pushfirst!(A, c...), a, b)
35343563

3564+
# sizehint! does not nothing by default
3565+
sizehint!(a::AbstractVector, _) = a
3566+
35353567
## hashing AbstractArray ##
35363568

35373569
const hash_abstractarray_seed = UInt === UInt64 ? 0x7e2d6fb6448beb77 : 0xd4514ce5

test/abstractarray.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,15 @@ using .Main.OffsetArrays
14361436
end
14371437
end
14381438

1439+
@testset "Check push!($a, $args...)" for
1440+
a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1)),
1441+
args in (("eenie",), ("eenie", "minie"), ("eenie", "minie", "mo"))
1442+
orig = copy(a)
1443+
push!(a, args...)
1444+
@test length(a) == length(orig) + length(args)
1445+
@test all(a[end-length(args)+1:end] .== args)
1446+
end
1447+
14391448
@testset "splatting into hvcat" begin
14401449
t = (1, 2)
14411450
@test [t...; 3 4] == [1 2; 3 4]

0 commit comments

Comments
 (0)