add sizehint! for first and make append!/prepend! safer

First we add an optional API parameter for `sizehint!` that controls
whether it is for `push!` (default) or `pushfirst!`. Secondly, we make
the offset zero when using `sizehint!` to shrink an array from the end,
or the trailing size zero when using it to shring from the beginning.

Then we replace the prior implementations of `prepend!` and `append!`
with ones that are more safe, even if the iterator changes length during
the operation or if convert fails. The result of `prepend!` may be in an
undefined order (because of the `reverse!` call) in the presence of
concurrent modifications or errors, but at least all of the elements
will be present and all entries will be valid afterwards.

Replaces and closes #49905
Replaces and closes #47391
Fixes #15868

Co-authored-by: Sukera <Seelengrab@users.noreply.github.com>
Co-authored-by: MasonProtter <mason.protter@icloud.com>

base/array.jl

@@ -1294,11 +1294,11 @@ and [`prepend!`](@ref) and [`pushfirst!`](@ref) for the opposite order.
 """
 function append! end
 
-function append!(a::Vector, items::AbstractVector)
-    itemindices = eachindex(items)
-    n = length(itemindices)
+function append!(a::Vector{T}, items::Union{AbstractVector{<:T},Tuple}) where T
+    items isa Tuple && (items = map(x -> convert(T, x), items))
+    n = length(items)
     _growend!(a, n)
-    copyto!(a, length(a)-n+1, items, first(itemindices), n)
+    copyto!(a, length(a)-n+1, items, firstindex(items), n)
     return a
 end
 
@@ -1308,16 +1308,11 @@ push!(a::AbstractVector, iter...) = append!(a, iter)
 append!(a::AbstractVector, iter...) = foldl(append!, iter, init=a)
 
 function _append!(a::AbstractVector, ::Union{HasLength,HasShape}, iter)
-    @_terminates_locally_meta
-    n = length(a)
+    n = Int(length(iter))::Int
     i = lastindex(a)
-    resize!(a, n+Int(length(iter))::Int)
-    for (i, item) in zip(i+1:lastindex(a), iter)
-        if isa(a, Vector) # give better effects for builtin vectors
-            @_safeindex a[i] = item
-        else
-            a[i] = item
-        end
+    sizehint!(a, length(a) + n; shrink=false)
+    for item in iter
+        push!(a, item)
     end
     a
 end
@@ -1359,15 +1354,13 @@ julia> prepend!([6], [1, 2], [3, 4, 5])
 """
 function prepend! end
 
-function prepend!(a::Vector, items::AbstractVector)
-    itemindices = eachindex(items)
-    n = length(itemindices)
+function prepend!(a::Vector{T}, items::Union{AbstractVector{<:T},Tuple}) where T
+    items isa Tuple && (items = map(x -> convert(T, x), items))
+    n = length(items)
     _growbeg!(a, n)
-    if a === items
-        copyto!(a, 1, items, n+1, n)
-    else
-        copyto!(a, 1, items, first(itemindices), n)
-    end
+    # in case of aliasing, the _growbeg might have shifted our data, so copy
+    # just the last n elements instead of all of them from the first
+    copyto!(a, 1, items, lastindex(items)-n+1, n)
     return a
 end
 
@@ -1379,12 +1372,14 @@ prepend!(a::AbstractVector, iter...) = foldr((v, a) -> prepend!(a, v), iter, ini
 function _prepend!(a::Vector, ::Union{HasLength,HasShape}, iter)
     @_terminates_locally_meta
     require_one_based_indexing(a)
-    n = length(iter)
-    _growbeg!(a, n)
-    i = 0
+    n = Int(length(iter))::Int
+    sizehint!(a, length(a) + n; first=true, shrink=false)
+    n = 0
     for item in iter
-        @_safeindex a[i += 1] = item
+        n += 1
+        pushfirst!(a, item)
     end
+    reverse!(a, 1, n)
     a
 end
 function _prepend!(a::Vector, ::IteratorSize, iter)
@@ -1441,13 +1436,17 @@ function resize!(a::Vector, nl::Integer)
 end
 
 """
-    sizehint!(s, n; shrink::Bool = true) -> s
+    sizehint!(s, n; first::Bool=false, shrink::Bool=true) -> s
 
 Suggest that collection `s` reserve capacity for at least `n` elements. That is, if
 you expect that you're going to have to push a lot of values onto `s`, you can avoid
 the cost of incremental reallocation by doing it once up front; this can improve
 performance.
 
+If `first` is true, then the reserved space is from the start of the collection, for ordered
+collections. Supplying this keyword may result in an error if the collection is nor ordered
+or if `pushfirst!` is not supported for this collection.
+
 See also [`resize!`](@ref).
 
 # Notes on the performance model
@@ -1466,40 +1465,51 @@ For types that support `sizehint!`,
 4. `shrink` controls if the collection can be shrunk.
 
 !!! compat "Julia 1.11"
-    The `shrink` argument was added in Julia 1.11.
+    The `shrink` and `first` arguments were added in Julia 1.11.
 """
 function sizehint! end
 
-function sizehint!(a::Vector, sz::Integer; shrink::Bool = true)
+function sizehint!(a::Vector, sz::Integer; first::Bool=false, shrink::Bool=true)
     len = length(a)
     ref = a.ref
     mem = ref.mem
     memlen = length(mem)
-    offset = memoryrefoffset(ref)
-    sz = max(Int(sz), offset + len - 1)
+    sz = max(Int(sz), len)
+    inc = sz - len
     if sz <= memlen
         # if we don't save at least 1/8th memlen then its not worth it to shrink
         if !shrink || memlen - sz <= div(memlen, 8)
             return a
         end
         newmem = array_new_memory(mem, sz)
-        if len == 0
-            newref = GenericMemoryRef(newmem)
+        if first
+            newref = GenericMemoryRef(newmem, inc + 1)
         else
-            unsafe_copyto!(newmem, offset, mem, offset, len)
-            newref = @inbounds GenericMemoryRef(newmem, offset)
+            newref = GenericMemoryRef(newmem)
         end
+        unsafe_copyto!(newref, ref, len)
         setfield!(a, :ref, newref)
-    else
-        inc = sz - len;
+    elseif first
+        _growbeg!(a, inc)
+        newref = getfield(a, :ref)
+        newref = GenericMemoryRef(newref, inc + 1)
+        setfield!(a, :size, (len,)) # undo the size change from _growbeg!
+        setfield!(a, :ref, newref) # undo the offset change from _growbeg!
+    else # last
         _growend!(a, inc)
         setfield!(a, :size, (len,)) # undo the size change from _growend!
     end
     a
 end
 
 # Fall-back implementation for non-shrinkable collections
-sizehint!(a, sz; shrink::Bool) = sizehint!(a, sz)
+# avoid defining this the normal way to avoid avoid infinite recursion
+function Core.kwcall(kwargs::NamedTuple{names}, ::typeof(sizehint!), a, sz) where names
+    get(kwargs, :first, false)::Bool
+    get(kwargs, :shrink, true)::Bool
+    isempty(diff_names(names, (:first, :shrink))) || kwerr(kwargs, sizehint!, a, sz)
+    sizehint!(a, sz)
+end
 
 """
     pop!(collection) -> item

base/bitset.jl

@@ -55,7 +55,10 @@ function copy!(dest::BitSet, src::BitSet)
     dest
 end
 
-sizehint!(s::BitSet, n::Integer) = (sizehint!(s.bits, (n+63) >> 6); s)
+function sizehint!(s::BitSet, n::Integer; first::Bool=false, shrink::Bool=true)
+    sizehint!(s.bits, (n+63) >> 6; first, shrink)
+    s
+end
 
 function _bits_getindex(b::Bits, n::Int, offset::Int)
     ci = _div64(n) - offset + 1

base/dict.jl

@@ -228,7 +228,7 @@ end
     return h
 end
 
-function sizehint!(d::Dict{T}, newsz; shrink::Bool = true) where T
+function sizehint!(d::Dict{T}, newsz; shrink::Bool=true) where T
     oldsz = length(d.slots)
     # limit new element count to max_values of the key type
     newsz = min(max(newsz, length(d)), max_values(T)::Int)
@@ -775,7 +775,7 @@ function map!(f, iter::ValueIterator{<:Dict})
 end
 
 function mergewith!(combine, d1::Dict{K, V}, d2::AbstractDict) where {K, V}
-    haslength(d2) && sizehint!(d1, length(d1) + length(d2), shrink = false)
+    haslength(d2) && sizehint!(d1, length(d1) + length(d2), shrink=false)
     for (k, v) in d2
         i, sh = ht_keyindex2_shorthash!(d1, k)
         if i > 0

base/set.jl

@@ -117,7 +117,7 @@ copymutable(s::Set{T}) where {T} = Set{T}(s)
 # Set is the default mutable fall-back
 copymutable(s::AbstractSet{T}) where {T} = Set{T}(s)
 
-sizehint!(s::Set, newsz; shrink::Bool = true) = (sizehint!(s.dict, newsz, shrink = shrink); s)
+sizehint!(s::Set, newsz; shrink::Bool=true) = (sizehint!(s.dict, newsz; shrink); s)
 empty!(s::Set) = (empty!(s.dict); s)
 rehash!(s::Set) = (rehash!(s.dict); s)
 

test/abstractarray.jl

@@ -1353,6 +1353,28 @@ Base.pushfirst!(tpa::TestPushArray{T}, a::T) where T = pushfirst!(tpa.data, a)
     @test tpa.data == reverse(collect(1:6))
 end
 
+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))
+
+isdefined(Main, :OffsetArrays) || @eval Main include("testhelpers/OffsetArrays.jl")
+using .Main.OffsetArrays
+
+@testset "Failing `$f` should not grow the array $a" for
+        f in (push!, append!, pushfirst!, prepend!),
+        a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1))
+    for args in ((1,), (1,2), ([1], [2]), [1])
+        orig = copy(a)
+        @test_throws Exception f(a, args...)
+        @test a == orig
+    end
+end
+
 @testset "splatting into hvcat" begin
     t = (1, 2)
     @test [t...; 3 4] == [1 2; 3 4]

test/arrayops.jl

@@ -1786,6 +1786,32 @@ end
     # offset array
     @test append!([1,2], OffsetArray([9,8], (-3,))) == [1,2,9,8]
     @test prepend!([1,2], OffsetArray([9,8], (-3,))) == [9,8,1,2]
+
+    # Error recovery
+    A = [1, 2]
+    @test_throws MethodError append!(A, [1, 2, "hi"])
+    @test A == [1, 2, 1, 2]
+
+    oA = OffsetVector(A, 0:3)
+    @test_throws InexactError append!(oA, [1, 2, 3.01])
+    @test oA == OffsetVector([1, 2, 1, 2, 1, 2], 0:5)
+
+    @test_throws InexactError append!(A, (x for x in [1, 2, 3.1]))
+    @test A == [1, 2, 1, 2, 1, 2, 1, 2]
+
+    @test_throws InexactError append!(A, (x for x in [1, 2, 3.1] if isfinite(x)))
+    @test A == [1, 2, 1, 2, 1, 2, 1, 2, 1, 2]
+
+    @test_throws MethodError prepend!(A, [1, 2, "hi"])
+    @test A == [2, 1, 1, 2, 1, 2, 1, 2, 1, 2, 1, 2]
+
+    A = [1, 2]
+    @test_throws InexactError prepend!(A, (x for x in [1, 2, 3.1]))
+    @test A == [2, 1, 1, 2]
+
+    A = [1, 2]
+    @test_throws InexactError prepend!(A, (x for x in [1, 2, 3.1] if isfinite(x)))
+    @test A == [2, 1, 1, 2]
 end
 
 let A = [1,2]