Skip to content
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.

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

searchsorted has an errant @inbounds #51176

Closed
dlfivefifty opened this issue Sep 4, 2023 · 2 comments
Closed

searchsorted has an errant @inbounds #51176

dlfivefifty opened this issue Sep 4, 2023 · 2 comments

Comments

@dlfivefifty
Copy link
Contributor

This should error:

julia> searchsorted([1,2], 2, 1, 10, Base.Order.ForwardOrdering())
2:3

When we run Julia with check-bounds = yes we get correct behaviour:

julia> searchsorted([1,2], 2, 1, 10, Base.Order.ForwardOrdering())
ERROR: BoundsError: attempt to access 2-element Vector{Int64} at index [5]
Stacktrace:
 [1] getindex
   @ ./essentials.jl:13 [inlined]
 [2] searchsorted(v::Vector{Int64}, x::Int64, ilo::Int64, ihi::Int64, o::Base.Order.ForwardOrdering)
   @ Base.Sort ./sort.jl:207
 [3] top-level scope
   @ REPL[1]:1

The problem applies to other searchsorted variants.

There should probably be a check on whether hi exceeds the length of a vector in

@inbounds while lo < hi - u

@gbaraldi gbaraldi added the bug Indicates an unexpected problem or unintended behavior label Sep 4, 2023
@LilithHafner
Copy link
Member

This is expected behavior. searchsorted([1,2], 2, 1, 10, Base.Order.ForwardOrdering()) is not part of the public API (because it is not mentioned in the documentation). You have called an internal method which assumes its arguments have already been boundschecked. It's unfortunate that this internal method shares the same function as the public searchsorted([1,2], 2).

@LilithHafner LilithHafner closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@LilithHafner LilithHafner removed the bug Indicates an unexpected problem or unintended behavior label Sep 4, 2023
@jishnub
Copy link
Contributor

jishnub commented Sep 4, 2023

This is expected behavior

Ideally, this shouldn't be the expected behavior, and the compiler, being able to prove that the indices are within bounds, should use inbounds without an explicit annotation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants