Bound Check when offsetting near `typemin/max(Int)`

[Moelf]

julia> a = rand(10);

julia> ao = OffsetArray(a, typemax(Int)-1)
10-element OffsetArray(::Array{Float64,1}, 9223372036854775807:-9223372036854775800) with eltype Float64 with indices 9223372036854775807:-9223372036854775800:

This looks like a bug?

[stevengj]

I suppose the OffsetArray constructor could check for integer overflow. (In general Julia does not perform overflow checking in Int computations, but in a constructor the overhead of a check is probably irrelevant.)

[johnnychen94]

Adding overflow check for this would increase the construction time for a significant amount of time:

struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N}
    parent::AA
    offsets::NTuple{N,Int}
    function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray}
        overflow_check.(axes(parent), offsets)
        new{T, N, AA}(parent, offsets)
    end
end

@inline function overflow_check(r, offset::T) where T
    if offset > 0 && last(r) > typemax(T) - offset
        throw(ArgumentError("offset $offset should be smaller than $(typemax(T) - offset)"))
    end
end

julia> @btime OffsetArray{Float64}(undef, -2:2, -2:2, -2:2, -2:2);
    91.145 ns (1 allocation: 5.06 KiB)

julia> @btime OffsetArray{Float64}(undef, -2:2, -2:2, -2:2, -2:2);
  106.296 ns (1 allocation: 5.06 KiB)

Unsure if this is acceptable.

[Moelf]

20% may seem large but this is on ns level so I personally would think this is okay. Also mostly because I don't believe people would construct OffsetArray for a few million times in tight loop (?)

but on the other hand, hitting overflow when making an OffsetArray in itself is super unlikely.

[jishnub]

This seems to perform somewhat better for me:

function overflow_check(r, offset::T) where T
    throw_offseterror() = throw(ArgumentError("offset $offset should be smaller than $(typemax(T) - offset)"))
    if offset > 0 && last(r) > typemax(T) - offset
        throw_offseterror()
    end
end

With this:

struct OffsetArray{T,N,AA<:AbstractArray} <: AbstractArray{T,N}
    parent::AA
    offsets::NTuple{N,Int}
    function OffsetArray{T, N, AA}(parent::AA, offsets::NTuple{N, Int}) where {T, N, AA<:AbstractArray}
        overflow_check.(axes(parent), offsets)
        new{T, N, AA}(parent, offsets)
    end
end

function overflow_check(r, offset::T) where T
    throw_offseterror() = throw(ArgumentError("offset $offset should be smaller than $(typemax(T) - offset)"))
    if offset > 0 && last(r) > typemax(T) - offset
        throw_offseterror()
    end
end

the run-time is

julia> a = zeros(5,5,5,5);

julia> @btime OffsetArray($a, -2:2, -2:2, -2:2, -2:2);
  33.271 ns (0 allocations: 0 bytes)

and with the bounds-checking disabled, the run-time is

julia> @btime OffsetArray($a, -2:2, -2:2, -2:2, -2:2);
  30.106 ns (0 allocations: 0 bytes)

The difference is around 10%.

Compared to the original, which produces

julia> @btime OffsetArray($a, -2:2, -2:2, -2:2, -2:2);
  41.650 ns (0 allocations: 0 bytes)