diff --git a/base/array.jl b/base/array.jl index a8d98be3a9b9f..d883c620428e4 100644 --- a/base/array.jl +++ b/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,32 +1465,37 @@ 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 @@ -1499,7 +1503,13 @@ function sizehint!(a::Vector, sz::Integer; shrink::Bool = true) 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 diff --git a/base/bitset.jl b/base/bitset.jl index 0e26fc28acbd0..78d8fc8769de1 100644 --- a/base/bitset.jl +++ b/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 diff --git a/base/dict.jl b/base/dict.jl index 7eef5e76e140a..26896c78a7008 100644 --- a/base/dict.jl +++ b/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 diff --git a/base/set.jl b/base/set.jl index 460f6f176f889..fb1f3d1959014 100644 --- a/base/set.jl +++ b/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) diff --git a/test/abstractarray.jl b/test/abstractarray.jl index 9167cfd954d5f..3ffaf8a2a2085 100644 --- a/test/abstractarray.jl +++ b/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] diff --git a/test/arrayops.jl b/test/arrayops.jl index 2691da4b17154..30f00ca5d5a46 100644 --- a/test/arrayops.jl +++ b/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]