Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 13, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f37e6c4
add test demonstrating overflow in countsort
Dec 7, 2022
2ea6508
fix overflow in countsort
Dec 7, 2022
a7602fb
convert back to signed without overflow error
Dec 7, 2022
34cad56
add another failing test
Dec 8, 2022
d8cadea
remove unnecessary type annotations (fixes tests)
Dec 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions base/sort.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions test/sorting.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down