From 5230d27de950165475892acfabded59713c8cd3e Mon Sep 17 00:00:00 2001
From: Mark Kittisopikul <mkitti@users.noreply.github.com>
Date: Fri, 16 Aug 2024 03:33:22 -0400
Subject: [PATCH] Fix push! for OffsetVectors, add tests for push! and append!
 on AbstractVector (#55480)

Per
https://github.com/JuliaLang/julia/pull/55470#discussion_r1714000529,
the `push!(::AbstractArray, ...)` array implementation assumed one-based
indexing and did not account for an `OffsetVector`
scenario.

Here we add tests for `push!(::AbstractArray, ...)` and
`append(::AbstractArray, ...)` including using `@invoke` to test the
effect on `OffsetVector`.

cc: @fredrikekre
---
 base/abstractarray.jl |  7 ++++---
 test/abstractarray.jl | 18 +++++++++++++++++-
 test/offsetarray.jl   | 29 +++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/base/abstractarray.jl b/base/abstractarray.jl
index 3f8886e14940c..754ab20660ab8 100644
--- a/base/abstractarray.jl
+++ b/base/abstractarray.jl
@@ -3532,7 +3532,7 @@ function push!(a::AbstractVector{T}, item) where T
     itemT = item isa T ? item : convert(T, item)::T
     new_length = length(a) + 1
     resize!(a, new_length)
-    a[new_length] = itemT
+    a[end] = itemT
     return a
 end
 
@@ -3540,7 +3540,7 @@ end
 function push!(a::AbstractVector{Any}, @nospecialize x)
     new_length = length(a) + 1
     resize!(a, new_length)
-    a[new_length] = x
+    a[end] = x
     return a
 end
 function push!(a::AbstractVector{Any}, @nospecialize x...)
@@ -3548,8 +3548,9 @@ function push!(a::AbstractVector{Any}, @nospecialize x...)
     na = length(a)
     nx = length(x)
     resize!(a, na + nx)
+    e = lastindex(a) - nx
     for i = 1:nx
-        a[na+i] = x[i]
+        a[e+i] = x[i]
     end
     return a
 end
diff --git a/test/abstractarray.jl b/test/abstractarray.jl
index 8b4a1d9113940..f655d9abe423f 100644
--- a/test/abstractarray.jl
+++ b/test/abstractarray.jl
@@ -1437,14 +1437,30 @@ using .Main.OffsetArrays
 end
 
 @testset "Check push!($a, $args...)" for
-    a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1)),
+    a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), SimpleArray{Any}(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1)),
     args in (("eenie",), ("eenie", "minie"), ("eenie", "minie", "mo"))
         orig = copy(a)
         push!(a, args...)
         @test length(a) == length(orig) + length(args)
+        @test a[axes(orig,1)] == orig
         @test all(a[end-length(args)+1:end] .== args)
 end
 
+@testset "Check append!($a, $args)" for
+    a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), SimpleArray{Any}(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1)),
+    args in (("eenie",), ("eenie", "minie"), ("eenie", "minie", "mo"))
+        orig = copy(a)
+        append!(a, args)
+        @test length(a) == length(orig) + length(args)
+        @test a[axes(orig,1)] == orig
+        @test all(a[end-length(args)+1:end] .== args)
+end
+
+@testset "Check sizehint!($a)" for
+    a in (["foo", "Bar"], SimpleArray(["foo", "Bar"]), SimpleArray{Any}(["foo", "Bar"]), OffsetVector(["foo", "Bar"], 0:1))
+        @test sizehint!(a, 10) === a
+end
+
 @testset "splatting into hvcat" begin
     t = (1, 2)
     @test [t...; 3 4] == [1 2; 3 4]
diff --git a/test/offsetarray.jl b/test/offsetarray.jl
index c50f38c382385..5ee918e85faf7 100644
--- a/test/offsetarray.jl
+++ b/test/offsetarray.jl
@@ -383,6 +383,18 @@ v2 = copy(v)
 @test v2[end-1] == 2
 @test v2[end] == 1
 
+# push!(v::AbstractVector, x...)
+v2 = copy(v)
+@test @invoke(push!(v2::AbstractVector, 3)) === v2
+@test v2[axes(v,1)] == v
+@test v2[end] == 3
+@test v2[begin] == v[begin] == v[-2]
+v2 = copy(v)
+@test @invoke(push!(v2::AbstractVector, 5, 6)) == v2
+@test v2[axes(v,1)] == v
+@test v2[end-1] == 5
+@test v2[end] == 6
+
 # append! from array
 v2 = copy(v)
 @test append!(v2, [2, 1]) === v2
@@ -399,6 +411,23 @@ v2 = copy(v)
 @test v2[axes(v, 1)] == v
 @test v2[lastindex(v)+1:end] == [2, 1]
 
+# append!(::AbstractVector, ...)
+# append! from array
+v2 = copy(v)
+@test @invoke(append!(v2::AbstractVector, [2, 1]::Any)) === v2
+@test v2[axes(v, 1)] == v
+@test v2[lastindex(v)+1:end] == [2, 1]
+# append! from HasLength iterator
+v2 = copy(v)
+@test @invoke(append!(v2::AbstractVector, (v for v in [2, 1])::Any)) === v2
+@test v2[axes(v, 1)] == v
+@test v2[lastindex(v)+1:end] == [2, 1]
+# append! from SizeUnknown iterator
+v2 = copy(v)
+@test @invoke(append!(v2::AbstractVector, (v for v in [2, 1] if true)::Any)) === v2
+@test v2[axes(v, 1)] == v
+@test v2[lastindex(v)+1:end] == [2, 1]
+
 # other functions
 v = OffsetArray(v0, (-3,))
 @test lastindex(v) == 1