Bug with range() and unsigned indices

[gerw]

The snippet

x = range(-1,1,length=11)
x[UInt(1)]

gives 3.6893488147419105e18 instead of the expected -1.0.

The problem seems to be

julia/base/range.jl

Line 939 in 0d29712

u = i - r.offset

, since UInt(1) - 6 underflows.

julia> versioninfo()
Julia Version 1.6.2
Commit 1b93d53fc4 (2021-07-14 15:36 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: Intel(R) Core(TM) i7-8700 CPU @ 3.20GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.1 (ORCJIT, skylake)

[Seelengrab]

Can confirm on (somewhat recent) master. I think there are more places out there hiding like that, like #42528 previously.. Finding those will be a little bit of a challenge :/

[gbaraldi]

I wonder how we could fix this. Casting it to an integer means that there would be indices that aren't representible. But ranges currently assume that you can have a negative index. I guess adding a special dispatch for unsigned integers that does some checking while leaving the normal integer one alone could work.

[Seelengrab]

Yeah, special casing Unsigned vs. Signed is sadly necessary, because of those assumptions about underflow on <: Integer.

Note that negative indices are still fine here, they can live happily in the Signed path, since there can't ever be a negative UInt anyway.

The same fix should be applied to

julia/base/range.jl

Lines 943 to 947 in 0d29712

	function _getindex_hiprec(r::StepRangeLen, i::Integer) # without rounding by T
	i isa Bool && throw(ArgumentError("invalid index: $i of type Bool"))
	u = i - r.offset
	r.ref + u*r.step
	end

as well.

[gbaraldi]

I was giving this a stab, the same functions in twiceprecision.jl also have to changed.
I had two ideas one is simply :

function unsafe_getindex(r::StepRangeLen{T}, i::Unsigned) where T
    i = Int(i)
    u = i - r.offset
    T(r.ref + u*r.step)
end

It does mean that very large unsigned ints will fail. Another idea is to put the type cast behind an if statement. But that would be type unstable.

[vtjnash]

My untested recommendation is (after bitter experience dealing with similar questions for computing length haha) is to force inbounds math via a branch here:

function unsafe_getindex(r::StepRangeLen{T}, i::Integer) where T
    i isa Bool && throw(ArgumentError("invalid index: $i of type Bool"))
    if i < r.offset # 1 <= offset <= len && 1 <= i <= len
        u = r.offset - i
        return T(r.ref - u*r.step)
    else
        u = i - r.offset
        return T(r.ref + u*r.step)
    end
end

and also needs to apply to

function getindex(r::StepRangeLen{T}, s::OrdinalRange{S}) where {T, S<:Integer}

[gbaraldi]

Would keeping the dispatch separated be better to avoid a branch on signed integers? Or would the performance difference not be that significant?

[gbaraldi]

There are a lot of cases where the same error happens. I will open a PR to get started.

[adienes]

could this possibly be tagged for 1.10? it's pretty easy to encounter; many combinations of range(x, y, z)[m] for x < 0, y > 0, and unsigned m will silently return undef values