Bugfix in overflow, CartesianIndices for BigInt ranges (#227)

* Bugfix in overflow, CartesianIndices for BigInt ranges

* limit parameters in AbstractUnitRange constrctors

* no overflow check for BigInt ranges

* conversion to OrdinalRange

* Add test

* no-op IdOffsetRange eltype coercion

* fix version lower limit

Author: jishnub

src/OffsetArrays.jl

@@ -136,7 +136,8 @@ Type alias and convenience constructor for two-dimensional [`OffsetArray`](@ref)
 const OffsetMatrix{T,AA<:AbstractMatrix{T}} = OffsetArray{T,2,AA}
 
 # checks if the offset may be added to the range without overflowing
-function overflow_check(r::AbstractUnitRange{T}, offset::T) where T<:Integer
+function overflow_check(r::AbstractUnitRange{T}, offset::Integer) where {T<:Integer}
+    Base.hastypemax(T) || return nothing
     # This gives some performance boost https://github.com/JuliaLang/julia/issues/33273
     throw_upper_overflow_error(val) = throw(OverflowError("offset should be <= $(typemax(T) - val) corresponding to the axis $r, received an offset $offset"))
     throw_lower_overflow_error(val) = throw(OverflowError("offset should be >= $(typemin(T) - val) corresponding to the axis $r, received an offset $offset"))
@@ -153,13 +154,14 @@ function overflow_check(r::AbstractUnitRange{T}, offset::T) where T<:Integer
     return nothing
 end
 # checks if the two offsets may be added together without overflowing
-function overflow_check(offset1::T, offset2::T) where {T<:Integer}
-    throw_upper_overflow_error() = throw(OverflowError("offset should be <= $(typemax(eltype(offset1)) - offset1) given a pre-existing offset of $offset1, received an offset $offset2"))
-    throw_lower_overflow_error() = throw(OverflowError("offset should be >= $(typemin(eltype(offset1)) - offset1) given a pre-existing offset of $offset1, received an offset $offset2"))
+function overflow_check(offset_preexisting::Integer, offset_new::T) where {T<:Integer}
+    Base.hastypemax(T) || return nothing
+    throw_upper_overflow_error() = throw(OverflowError("offset should be <= $(typemax(eltype(offset_preexisting)) - offset_preexisting) given a pre-existing offset of $offset_preexisting, received an offset $offset_new"))
+    throw_lower_overflow_error() = throw(OverflowError("offset should be >= $(typemin(eltype(offset_preexisting)) - offset_preexisting) given a pre-existing offset of $offset_preexisting, received an offset $offset_new"))
 
-    if offset1 > 0 && offset2 > typemax(T) - offset1
+    if offset_preexisting > 0 && offset_new > typemax(T) - offset_preexisting
         throw_upper_overflow_error()
-    elseif offset1 < 0 && offset2 < typemin(T) - offset1
+    elseif offset_preexisting < 0 && offset_new < typemin(T) - offset_preexisting
         throw_lower_overflow_error()
     end
     return nothing

src/axes.jl

@@ -106,6 +106,7 @@ function IdOffsetRange{T,I}(r::IdOffsetRange, offset::Integer = 0) where {T<:Int
     rc, offset_rc = offset_coerce(I, r.parent)
     return IdOffsetRange{T,I}(rc, convert(T, r.offset + offset + offset_rc)::T)
 end
+IdOffsetRange{T}(r::IdOffsetRange{T}) where {T<:Integer} = r
 function IdOffsetRange{T}(r::IdOffsetRange, offset::Integer = 0) where T<:Integer
     return IdOffsetRange{T}(r.parent, r.offset + offset)
 end
@@ -118,6 +119,15 @@ function IdOffsetRange(; values::AbstractUnitRange{<:Integer}, indices::Abstract
     return IdOffsetRange(values .- offset, offset)
 end
 
+# Conversions to an AbstractUnitRange{Int} (and to an OrdinalRange{Int,Int} on Julia v"1.6") are necessary
+# to evaluate CartesianIndices for BigInt ranges, as their axes are also BigInt ranges
+AbstractUnitRange{T}(r::IdOffsetRange) where {T<:Integer} = IdOffsetRange{T}(r)
+
+# A version upper bound on this may be set after https://github.com/JuliaLang/julia/pull/40038 is merged
+if v"1.6" <= VERSION
+    OrdinalRange{T,T}(r::IdOffsetRange) where {T<:Integer} = IdOffsetRange{T}(r)
+end
+
 # TODO: uncomment these when Julia is ready
 # # Conversion preserves both the values and the indices, throwing an InexactError if this
 # # is not possible.

test/runtests.jl

@@ -245,6 +245,19 @@ end
     @test typeof(rred) == typeof(r)
     @test length(rred) == 1
     @test first(rred) == first(r)
+
+    @testset "conversion to AbstractUnitRange" begin
+        r = IdOffsetRange(1:2)
+        @test AbstractUnitRange{Int}(r) === r
+        r2 = IdOffsetRange(big(1):big(2))
+        @test AbstractUnitRange{Int}(r2) === r
+        @test AbstractUnitRange{BigInt}(r2) === r2
+
+        if v"1.5" < VERSION
+            @test OrdinalRange{Int,Int}(r2) === r
+            @test OrdinalRange{BigInt,BigInt}(r2) === r2
+        end
+    end
 end
 
 # used in testing the constructor
@@ -395,12 +408,20 @@ Base.convert(::Type{Int}, a::WeirdInteger) = a
         @test_throws OverflowError OffsetArray(ao, (-2, )) # convinient constructor accumulate offsets
         @test_throws OverflowError OffsetVector(1:0, typemax(Int))
         @test_throws OverflowError OffsetVector(OffsetVector(1:0, 0), typemax(Int))
+        @test_throws OverflowError OffsetArray(zeros(Int, typemax(Int):typemax(Int)), 2)
 
         @testset "OffsetRange" begin
-            local r = 1:100
-            local a = OffsetVector(r, 4)
-            @test first(r) in a
-            @test !(last(r) + 1 in a)
+            for r in Any[1:100, big(1):big(2)]
+                a = OffsetVector(r, 4)
+                @test first(r) in a
+                @test !(last(r) + 1 in a)
+            end
+
+            @testset "BigInt axes" begin
+                r = OffsetArray(1:big(2)^65, 4000)
+                @test eltype(r) === BigInt
+                @test axes(r, 1) == (big(1):big(2)^65) .+ 4000
+            end
         end
 
         # disallow OffsetVector(::Array{<:Any, N}, offsets) where N != 1
@@ -765,6 +786,12 @@ end
     @test eachindex(IndexLinear(), S) == eachindex(IndexLinear(), A0)
     A = ones(5:6)
     @test eachindex(IndexLinear(), A) === axes(A, 1)
+
+    A = OffsetArray(big(1):big(2), 1)
+    B = OffsetArray(1:2, 1)
+    @test CartesianIndices(A) == CartesianIndices(B)
+    @test LinearIndices(A) == LinearIndices(B)
+    @test eachindex(A) == eachindex(B)
 end
 
 @testset "Scalar indexing" begin