Fix indexing with Slice (#210)

* Fix indexing with Slice

* remove trailing whitespace

* Add test for IdOffsetRange(::Base.OneTo)

* specify array eltypes as Any in tests

Author: jishnub

src/OffsetArrays.jl

@@ -349,17 +349,17 @@ end
 
 for OR in [:IIUR, :IdOffsetRange]
     for R in [:StepRange, :StepRangeLen, :LinRange, :UnitRange]
-        @eval @propagate_inbounds Base.getindex(r::$R, s::$OR) = OffsetArray(r[no_offset_view(s)], axes(s))
+        @eval @propagate_inbounds Base.getindex(r::$R, s::$OR) = OffsetArray(r[UnitRange(s)], axes(s))
     end
 
     # this method is needed for ambiguity resolution
     @eval @propagate_inbounds Base.getindex(r::StepRangeLen{T,<:Base.TwicePrecision,<:Base.TwicePrecision}, s::$OR) where T =
-    OffsetArray(r[no_offset_view(s)], axes(s))
+    OffsetArray(r[UnitRange(s)], axes(s))
 
     #= Integer UnitRanges may return an appropriate AbstractUnitRange{<:Integer}, as the result may be used in indexing, and
     indexing is faster with ranges =#
     @eval @propagate_inbounds function Base.getindex(r::UnitRange{<:Integer}, s::$OR)
-        rs = r[no_offset_view(s)]
+        rs = r[UnitRange(s)]
         offset_s = first(axes(s,1)) - 1
         IdOffsetRange(UnitRange(rs .- offset_s), offset_s)
     end

test/runtests.jl

@@ -817,7 +817,7 @@ end
     r2 = r1[:]
     @test r2 == r1
 
-    for r1 in [
+    for r1 in Any[
         # AbstractArrays
         OffsetArray(10:1000, 0), # 1-based index
         OffsetArray(10:3:1000, 3), # offset index
@@ -829,9 +829,12 @@ end
         OffsetArray(IdOffsetRange(IdOffsetRange(10:1000, -1), 1), 3), # offset index
 
         # AbstractRanges
-        1:1000, 
-        1:3:1000, 
+        1:1000,
+        UnitRange(1.0, 1000.0),
+        1:3:1000,
         1.0:3.0:1000.0,
+        StepRangeLen(Float64(1), Float64(1000), 1000),
+        LinRange(1, 1000, 1000),
         IdOffsetRange(ZeroBasedUnitRange(1:1000), 1), # 1-based index
         IdOffsetRange(ZeroBasedUnitRange(1:1000), 2), # offset index
         ZeroBasedUnitRange(1:1000), # offset range
@@ -840,27 +843,52 @@ end
         ]
 
         # AbstractArrays with 1-based indices
-        for r2 in [
-            OffsetArray(5:80, 0), 
-            OffsetArray(5:2:80, 0), 
-            OffsetArray(IdentityUnitRange(5:80), -4), 
+        for r2 in Any[
+            OffsetArray(5:80, 0),
+            OffsetArray(5:2:80, 0),
+            OffsetArray(IdentityUnitRange(5:80), -4),
             OffsetArray(IdOffsetRange(5:80), 0),
             ]
 
             test_indexing_axes_and_vals(r1, r2)
         end
 
         # AbstractRanges with 1-based indices
-        for r2 in [
-            5:80, 
-            5:2:80, 
-            IdOffsetRange(5:80), 
+        for r2 in Any[
+            5:80,
+            5:2:80,
+            IdOffsetRange(5:80),
             IdOffsetRange(ZeroBasedUnitRange(4:79), 1),
             ]
 
             test_indexing_axes_and_vals(r1, r2)
         end
     end
+
+    # Indexing with IdentityUnitRange(::Base.OneTo) or Base.Slice(::OneTo) is special.
+    # This is because axes(::IdentityUnitRange{<:Base.OneTo}, 1) isa Base.OneTo, and not an IdentityUnitRange.
+    # These therefore may pass through no_offset_view unchanged.
+    # This had led to a stack-overflow in indexing, as getindex was using no_offset_view.
+    # Issue 209
+    for r1 in Any[
+        # This set of tests is for ranges r1 that have 1-based indices
+        UnitRange(1.0, 99.0),
+        1:99,
+        1:1:99,
+        1.0:1.0:99.0,
+        StepRangeLen(Float64(1), Float64(99), 99),
+        LinRange(1, 99, 99),
+        ]
+
+        for r2 in Any[
+            IdentityUnitRange(Base.OneTo(10)),
+            Base.Slice(Base.OneTo(10)),
+            IdOffsetRange(Base.OneTo(10)),
+            ]
+
+            test_indexing_axes_and_vals(r1, r2)
+        end
+    end
 end
 
 @testset "Vector indexing with offset ranges" begin
@@ -887,7 +915,7 @@ end
         @test a[ax[i]] == a[ax][i]
     end
 
-    for r1 in [
+    for r1 in Any[
         # AbstractArrays
         OffsetArray(10:1000, 0), # 1-based index
         OffsetArray(10:1000, 3), # offset index
@@ -902,8 +930,10 @@ end
 
         # AbstractRanges
         1:1000,
+        UnitRange(1.0, 1000.0),
         1:2:2000,
         1.0:2.0:2000.0,
+        StepRangeLen(Float64(1), Float64(1000), 1000),
         LinRange(1.0, 2000.0, 2000),
         IdOffsetRange(1:1000, 0), # 1-based index
         IdOffsetRange(ZeroBasedUnitRange(1:1000), 1), # 1-based index
@@ -916,9 +946,10 @@ end
         ]
 
         # AbstractArrays with offset axes
-        for r2 in [OffsetArray(5:80, 40), OffsetArray(5:2:80, 40), 
-            OffsetArray(IdentityUnitRange(5:80), 2), 
-            OffsetArray(IdOffsetRange(5:80, 1), 3), 
+        for r2 in Any[OffsetArray(5:80, 40),
+            OffsetArray(5:2:80, 40),
+            OffsetArray(IdentityUnitRange(5:80), 2),
+            OffsetArray(IdOffsetRange(5:80, 1), 3),
             OffsetArray(IdOffsetRange(IdOffsetRange(5:80, 4), 1), 3),
             OffsetArray(IdOffsetRange(IdentityUnitRange(5:80), 1), 3),
             OffsetArray(IdentityUnitRange(IdOffsetRange(5:80, 1)), 3),
@@ -928,11 +959,12 @@ end
         end
 
         # AbstractRanges with offset axes
-        for r2 in [IdOffsetRange(5:80, 1),
+        for r2 in Any[IdOffsetRange(5:80, 1),
             IdentityUnitRange(5:80),
-            IdOffsetRange(IdOffsetRange(5:80, 2), 1), 
+            IdOffsetRange(Base.OneTo(9), 4),
+            IdOffsetRange(IdOffsetRange(5:80, 2), 1),
             IdOffsetRange(IdOffsetRange(IdOffsetRange(5:80, -1), 2), 1),
-            IdentityUnitRange(IdOffsetRange(1:10, 5)), 
+            IdentityUnitRange(IdOffsetRange(1:10, 5)),
             IdOffsetRange(IdentityUnitRange(15:20), -2),
             ZeroBasedUnitRange(5:80),
             ZeroBasedRange(5:80),
@@ -1189,7 +1221,7 @@ end
     @test String(take!(io)) == "3:5 with indices 0:2"
 
     # issue #198
-    for r in [axes(OffsetVector(1:10, -5), 1), 1:1:2, 1.0:1.0:2.0, 1:-1:-5]
+    for r in Any[axes(OffsetVector(1:10, -5), 1), 1:1:2, 1.0:1.0:2.0, 1:-1:-5]
         a = OffsetVector(r, 5)
         show(io, a)
         @test String(take!(io)) == "$r with indices $(UnitRange(axes(a,1)))"