Skip to content

Commit

Permalink
Fix and test an overflow issue in searchsorted (#56464)
Browse files Browse the repository at this point in the history
And remove `searchsorted` special cases for offset arrays in tests that
had the impact of bypassing actually testing `searchsorted` behavior on
offset arrays

To be clear, after this bugfix the function is still broken, just a little bit less so.
  • Loading branch information
LilithHafner authored Nov 5, 2024
1 parent 9af0dea commit cbcad6f
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 13 deletions.
4 changes: 2 additions & 2 deletions base/sort.jl
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,8 @@ function searchsorted(v::AbstractVector, x, ilo::T, ihi::T, o::Ordering)::UnitRa
elseif lt(o, x, v[m])
hi = m
else
a = searchsortedfirst(v, x, max(lo,ilo), m, o)
b = searchsortedlast(v, x, m, min(hi,ihi), o)
a = searchsortedfirst(v, x, lo+u, m, o)
b = searchsortedlast(v, x, m, hi-u, o)
return a : b
end
end
Expand Down
4 changes: 4 additions & 0 deletions test/sorting.jl
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,10 @@ end
@test searchsortedfirst(o, 1.5) == 0
@test searchsortedlast(o, 0) == firstindex(o) - 1
@test searchsortedlast(o, 1.5) == -1

# Issue #56457
o2 = OffsetArray([2,2,3], typemax(Int)-3);
@test searchsorted(o2, 2) == firstindex(o2):firstindex(o2)+1
end

function adaptive_sort_test(v; trusted=InsertionSort, kw...)
Expand Down
11 changes: 0 additions & 11 deletions test/testhelpers/OffsetArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -821,17 +821,6 @@ centered(A::AbstractArray, cp::Dims=center(A)) = OffsetArray(A, .-cp)

centered(A::AbstractArray, i::CartesianIndex) = centered(A, Tuple(i))

# we may pass the searchsorted* functions to the parent, and wrap the offset
for f in [:searchsortedfirst, :searchsortedlast, :searchsorted]
_safe_f = Symbol("_safe_" * String(f))
@eval function $_safe_f(v::OffsetArray, x, ilo, ihi, o::Base.Ordering)
offset = firstindex(v) - firstindex(parent(v))
$f(parent(v), x, ilo - offset, ihi - offset, o) .+ offset
end
@eval Base.$f(v::OffsetVector, x, ilo::T, ihi::T, o::Base.Ordering) where T<:Integer =
$_safe_f(v, x, ilo, ihi, o)
end

##
# Deprecations
##
Expand Down

0 comments on commit cbcad6f

Please sign in to comment.