Fixups for #47383 (fixes runbenchmarks("sort")) (#47822)

* add test demonstrating overflow in countsort * fix overflow in countsort * remove unnecessary type annotations (fixes tests) This fixes the test failure because it allows for automatic conversion. The manual for implementing the AbstractArray interface also does not recomend a type signature for the value arg in setindex!. Co-authored-by: Lilith Hafner <[email protected]> (cherry picked from commit 965bc7d)
JuliaLang · Dec 14, 2022 · 3d80653 · 3d80653
1 parent 0b845b1
commit 3d80653
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/base/sort.jl b/base/sort.jl
@@ -509,12 +509,12 @@ struct WithoutMissingVector{T, U} <: AbstractVector{T}
         new{nonmissingtype(eltype(data)), typeof(data)}(data)
     end
 end
-Base.@propagate_inbounds function Base.getindex(v::WithoutMissingVector, i::Integer)
+Base.@propagate_inbounds function Base.getindex(v::WithoutMissingVector, i)
     out = v.data[i]
     @assert !(out isa Missing)
     out::eltype(v)
 end
-Base.@propagate_inbounds function Base.setindex!(v::WithoutMissingVector{T}, x::T, i) where T
+Base.@propagate_inbounds function Base.setindex!(v::WithoutMissingVector, x, i)
     v.data[i] = x
     v
 end
@@ -830,7 +830,7 @@ maybe_reverse(o::ForwardOrdering, x) = x
 maybe_reverse(o::ReverseOrdering, x) = reverse(x)
 function _sort!(v::AbstractVector{<:Integer}, ::CountingSort, o::DirectOrdering, kw)
     @getkw lo hi mn mx scratch
-    range = o === Reverse ? mn-mx : mx-mn
+    range = maybe_unsigned(o === Reverse ? mn-mx : mx-mn)
     offs = 1 - (o === Reverse ? mx : mn)
 
     counts = fill(0, range+1) # TODO use scratch (but be aware of type stability)
@@ -843,7 +843,7 @@ function _sort!(v::AbstractVector{<:Integer}, ::CountingSort, o::DirectOrdering,
         lastidx = idx + counts[i] - 1
         val = i-offs
         for j = idx:lastidx
-            v[j] = val
+            v[j] = val isa Unsigned && eltype(v) <: Signed ? signed(val) : val
         end
         idx = lastidx + 1
     end

diff --git a/test/sorting.jl b/test/sorting.jl
@@ -765,6 +765,7 @@ end
 
 @testset "Unions with missing" begin
     @test issorted(sort(shuffle!(vcat(fill(missing, 10), rand(Int, 100)))))
+    @test issorted(sort(vcat(rand(Int8, 600), [missing])))
 end
 
 @testset "Specific algorithms" begin
@@ -897,6 +898,7 @@ end
 @testset "Count sort near the edge of its range" begin
     @test issorted(sort(rand(typemin(Int):typemin(Int)+100, 1000)))
     @test issorted(sort(rand(typemax(Int)-100:typemax(Int), 1000)))
+    @test issorted(sort(rand(Int8, 600)))
 end
 
 # This testset is at the end of the file because it is slow.