Skip to content

Commit

Permalink
Merge pull request #40364 from JuliaLang/jn/searchsorted-unsigned-bug
Browse files Browse the repository at this point in the history 
searchsorted ranges: simplify code, fix bug with Unsigned needle
  • Loading branch information
@vtjnash
vtjnash authored Apr 8, 2021
2 parents 5b6edad + 40d4f4a commit d3b012b
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 75 deletions.
4 changes: 2 additions & 2 deletions base/range.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ function argmin(r::AbstractRange)
elseif step(r) > 0
firstindex(r)
else
first(searchsorted(r, last(r)))
lastindex(r)
end
end

Expand All @@ -720,7 +720,7 @@ function argmax(r::AbstractRange)
if isempty(r)
throw(ArgumentError("range must be non-empty"))
elseif step(r) > 0
first(searchsorted(r, last(r)))
lastindex(r)
else
firstindex(r)
end
Expand Down
82 changes: 26 additions & 56 deletions base/sort.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,92 +231,62 @@ end

function searchsortedlast(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
if step(a) == 0
lt(o, x, first(a)) ? 0 : length(a)
f, h, l = first(a), step(a), last(a)
if lt(o, x, f)
0
elseif h == 0 || !lt(o, x, l)
length(a)
else
n = round(Integer, clamp((x - first(a)) / step(a) + 1, 1, length(a)))
n = round(Integer, (x - f) / h + 1)
lt(o, x, a[n]) ? n - 1 : n
end
end

function searchsortedfirst(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
if step(a) == 0
lt(o, first(a), x) ? length(a) + 1 : 1
f, h, l = first(a), step(a), last(a)
if !lt(o, f, x)
1
elseif h == 0 || lt(o, l, x)
length(a) + 1
else
n = round(Integer, clamp((x - first(a)) / step(a) + 1, 1, length(a)))
n = round(Integer, (x - f) / h + 1)
lt(o, a[n], x) ? n + 1 : n
end
end

function searchsortedlast(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
h = step(a)
if h == 0
lt(o, x, first(a)) ? 0 : length(a)
elseif h > 0 && x < first(a)
firstindex(a) - 1
elseif h > 0 && x >= last(a)
lastindex(a)
elseif h < 0 && x > first(a)
firstindex(a) - 1
elseif h < 0 && x <= last(a)
lastindex(a)
f, h, l = first(a), step(a), last(a)
if lt(o, x, f)
0
elseif h == 0 || !lt(o, x, l)
length(a)
else
if o isa ForwardOrdering
fld(floor(Integer, x) - first(a), h) + 1
fld(floor(Integer, x) - f, h) + 1
else
fld(ceil(Integer, x) - first(a), h) + 1
fld(ceil(Integer, x) - f, h) + 1
end
end
end

function searchsortedfirst(a::AbstractRange{<:Integer}, x::Real, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
h = step(a)
if h == 0
lt(o, first(a), x) ? length(a)+1 : 1
elseif h > 0 && x <= first(a)
firstindex(a)
elseif h > 0 && x > last(a)
lastindex(a) + 1
elseif h < 0 && x >= first(a)
firstindex(a)
elseif h < 0 && x < last(a)
lastindex(a) + 1
f, h, l = first(a), step(a), last(a)
if !lt(o, f, x)
1
elseif h == 0 || lt(o, l, x)
length(a) + 1
else
if o isa ForwardOrdering
-fld(floor(Integer, -x) + Signed(first(a)), h) + 1
cld(ceil(Integer, x) - f, h) + 1
else
-fld(ceil(Integer, -x) + Signed(first(a)), h) + 1
cld(floor(Integer, x) - f, h) + 1
end
end
end

function searchsortedfirst(a::AbstractRange{<:Integer}, x::Unsigned, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
if lt(o, first(a), x)
if step(a) == 0
length(a) + 1
else
min(cld(x - first(a), step(a)), length(a)) + 1
end
else
1
end
end

function searchsortedlast(a::AbstractRange{<:Integer}, x::Unsigned, o::DirectOrdering)::keytype(a)
require_one_based_indexing(a)
if lt(o, x, first(a))
0
elseif step(a) == 0
length(a)
else
min(fld(x - first(a), step(a)) + 1, length(a))
end
end

searchsorted(a::AbstractRange{<:Real}, x::Real, o::DirectOrdering) =
searchsortedfirst(a, x, o) : searchsortedlast(a, x, o)

Expand Down
45 changes: 28 additions & 17 deletions test/sorting.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ end
@test searchsorted(fill(R(1), 15), T(1), 6, 10, Forward) == 6:10
end

for (rg,I) in [(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)]
for (rg,I) in Any[(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)]
rg_r = reverse(rg)
rgv, rgv_r = [rg;], [rg_r;]
for i = I
Expand Down Expand Up @@ -144,7 +144,7 @@ end

@testset "issue 32568" begin
for R in numTypes, T in numTypes
for arr in [R[1:5;], R(1):R(5), R(1):2:R(5)]
for arr in Any[R[1:5;], R(1):R(5), R(1):2:R(5)]
@test eltype(searchsorted(arr, T(2))) == keytype(arr)
@test eltype(searchsorted(arr, T(2), big(1), big(4), Forward)) == keytype(arr)
@test searchsortedfirst(arr, T(2)) isa keytype(arr)
Expand All @@ -164,35 +164,46 @@ end
@test searchsorted([1,2], Inf) === 3:2
@test searchsorted(1:2, Inf) === 3:2

for coll in [
for coll in Any[
Base.OneTo(10),
1:2,
0x01:0x02,
-4:6,
5:2:10,
[1,2],
1.0:4,
[10.0,20.0],
]
for huge in [Inf, 1e300]
for huge in Any[Inf, 1e300, typemax(Int64), typemax(UInt64)]
@test searchsortedfirst(coll, huge) === lastindex(coll) + 1
@test searchsortedfirst(coll, -huge)=== firstindex(coll)
@test searchsortedlast(coll, huge) === lastindex(coll)
@test searchsortedlast(coll, -huge) === firstindex(coll) - 1
@test searchsorted(coll, huge) === lastindex(coll)+1 : lastindex(coll)
@test searchsorted(coll, -huge) === firstindex(coll) : firstindex(coll) - 1

@test searchsortedfirst(reverse(coll), huge, rev=true) === firstindex(coll)
@test searchsortedfirst(reverse(coll), -huge, rev=true) === lastindex(coll) + 1
@test searchsortedlast(reverse(coll), huge, rev=true) === firstindex(coll) - 1
@test searchsortedlast(reverse(coll), -huge, rev=true) === lastindex(coll)
@test searchsorted(reverse(coll), huge, rev=true) === firstindex(coll):firstindex(coll) - 1
@test searchsorted(reverse(coll), -huge, rev=true) === lastindex(coll)+1:lastindex(coll)
if !(eltype(coll) <: Unsigned)
@test searchsortedfirst(reverse(coll), huge, rev=true) === firstindex(coll)
@test searchsortedlast(reverse(coll), huge, rev=true) === firstindex(coll) - 1
@test searchsorted(reverse(coll), huge, rev=true) === firstindex(coll):firstindex(coll) - 1
end

if !(huge isa Unsigned)
@test searchsortedfirst(coll, -huge)=== firstindex(coll)
@test searchsortedlast(coll, -huge) === firstindex(coll) - 1
@test searchsorted(coll, -huge) === firstindex(coll) : firstindex(coll) - 1
if !(eltype(coll) <: Unsigned)
@test searchsortedfirst(reverse(coll), -huge, rev=true) === lastindex(coll) + 1
@test searchsortedlast(reverse(coll), -huge, rev=true) === lastindex(coll)
@test searchsorted(reverse(coll), -huge, rev=true) === lastindex(coll)+1:lastindex(coll)
end
end
end
end
@testset "issue ##34408" begin

@test_broken length(reverse(0x1:0x2)) == 2
@testset "issue #34408" begin
r = 1f8-10:1f8
# collect(r) = Float32[9.999999e7, 9.999999e7, 9.999999e7, 9.999999e7, 1.0e8, 1.0e8, 1.0e8, 1.0e8, 1.0e8]
@test_broken searchsorted(collect(r)) == searchsorted(r)
for i in r
@test_broken searchsorted(collect(r), i) == searchsorted(r, i)
end
end
end
@testset "issue #35272" begin
Expand Down Expand Up @@ -329,7 +340,7 @@ end
@test insorted(T(10), R.(collect(1:10)), by=(>=(5)))
end

for (rg,I) in [(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)]
for (rg,I) in Any[(49:57,47:59), (1:2:17,-1:19), (-3:0.5:2,-5:.5:4)]
rg_r = reverse(rg)
rgv, rgv_r = collect(rg), collect(rg_r)
for i = I
Expand Down

0 comments on commit d3b012b

Please sign in to comment.