Add push! implementation for AbstractArray depending only on resize!

[mkitti]

Fix #55459

In Julia 1.10, push! and append! would be functional for AbstractVector implementations
if resize! and setindex! were defined.

As of #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:

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> 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> 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> 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> push!(SimpleArray{Int}(zeros(Int, 5), 6)
6-element SimpleArray{Int64}:
 0
 0
 0
 0
 0
 6

[fredrikekre]

Unrelated change? Also no tests.

[mkitti]

#51903 introduced a dependency on sizehint! for append! that did not previously exist.

I will add tests for append.

[fredrikekre]

julia> o = OffsetVector([1, 2, 3], -1);

julia> @invoke push!(o::AbstractVector, 4::Any)
ERROR: BoundsError: attempt to access 4-element OffsetArray(::Vector{Int64}, 0:3) with eltype Int64 with indices 0:3 at index [4]

[mkitti]

I think this re-establishes the state before #51903 but I agree this is an issue.

Would end work better here or should we require or dispatch on one-based indexing?

[longemen3000]

on the specific case of OffsetArrays, resize! works on the the parent array (so length(a) + 1 is fine)

[mkitti]

I'm not sure I understand your point.

[KristofferC]

https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/13d440d_vs_34c3a63/FlexiMaps.primary.log

ERROR: LoadError: MethodError: no method matching resize!(::FlexiMaps.MappedArray{Int64, 1, Base.Fix2{typeof(+), Int64}, Vector{Int64}}, ::Int64)
The function `resize!` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  resize!(!Matched::JLArrays.JLArray{T, 1}, ::Integer) where T
   @ JLArrays ~/.julia/packages/JLArrays/hD5YX/src/JLArrays.jl:375
  resize!(!Matched::StructArrays.StructArray, ::Integer)
   @ StructArrays ~/.julia/packages/StructArrays/CjQ4L/src/structarray.jl:429
  resize!(!Matched::OffsetArrays.OffsetVector{T} where T, ::Integer)
   @ OffsetArrays ~/.julia/packages/OffsetArrays/hwmnB/src/OffsetArrays.jl:606
  ...

Stacktrace:
  [1] push!(a::FlexiMaps.MappedArray{Int64, 1, Base.Fix2{typeof(+), Int64}, Vector{Int64}}, item::Int64)
    @ Base ./abstractarray.jl:3506
  [2] top-level scope
    @ ~/.julia/packages/FlexiMaps/n1CYU/test/runtests.jl:226
  [3] eval
    @ ./boot.jl:430 [inlined]

https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/13d440d_vs_34c3a63/ZippedArrays.primary.log

zipped arrays: Test Failed at /home/pkgeval/.julia/packages/ZippedArrays/p7ZPn/test/runtests.jl:311
  Expression: sizehint!(ZippedVector(a, b, c, 1:n), n + 50)
    Expected: Exception
  No exception thrown

Are these related to this PR?

[mkitti]

FlexiMaps defines Base.append!(A::MappedArray, iter). push!(::MappedArray, ...) used to forward to append!(::MappedArray, ...). After this PR, push! no longer forwards to append!. Perhaps FlexiMaps should implement push! directly?

The second one with ZippedVector used to throw an exception to the MethodError below for UnitRange. I'm not sure if this should be fixed or not. This pull request provided a default implementation which just ignores the hint.

sizehint!(ZippedVector(a,b,c,1:n), n + 50)
ERROR: MethodError: no method matching sizehint!(::UnitRange{Int64}, ::Int64)