Merge pull request #59 from JuliaArrays/teh/unsafe_getindex

Don't extend Base.unsafe_getindex

Author: timholy

src/core.jl

@@ -60,7 +60,6 @@ axistype{name,T}(::Type{Axis{name,T}}) = T
 # Pass indexing and related functions straight through to the wrapped value
 # TODO: should Axis be an AbstractArray? AbstractArray{T,0} for scalar T?
 Base.getindex(A::Axis, i...) = A.val[i...]
-Base.unsafe_getindex(A::Axis, i...) = Base.unsafe_getindex(A, i...)
 Base.eltype{_,T}(::Type{Axis{_,T}}) = eltype(T)
 Base.size(A::Axis) = size(A.val)
 Base.indices(A::Axis) = indices(A.val)

src/indexing.jl

@@ -1,6 +1,6 @@
 const Idx = Union{Real,Colon,AbstractArray{Int}}
 
-using Base: ViewIndex, unsafe_getindex, unsafe_setindex!
+using Base: ViewIndex
 
 # Defer IndexStyle to the wrapped array
 @compat Base.IndexStyle{T,N,D,Ax}(::Type{AxisArray{T,N,D,Ax}}) = IndexStyle(D)
@@ -162,20 +162,20 @@ function axisindexes(::Type{Dimensional}, ax::Range, idx::RepeatedInterval)
     n = length(idx.offsets)
     idxs = unsafe_searchsorted(ax, idx.window)
     offsets = [searchsortednearest(ax, idx.offsets[i]) for i=1:n]
-    AxisArray(RepeatedRangeMatrix(idxs, offsets), Axis{:sub}(unsafe_getindex(ax, idxs)), Axis{:rep}(ax[offsets]))
+    AxisArray(RepeatedRangeMatrix(idxs, offsets), Axis{:sub}(inbounds_getindex(ax, idxs)), Axis{:rep}(ax[offsets]))
 end
 
 # We also have special datatypes to represent intervals about indices
 axisindexes(::Type{Dimensional}, ax::AbstractVector, idx::IntervalAtIndex) = searchsorted(ax, idx.window + ax[idx.index])
 function axisindexes(::Type{Dimensional}, ax::Range, idx::IntervalAtIndex)
     idxs = unsafe_searchsorted(ax, idx.window)
-    AxisArray(idxs + idx.index, Axis{:sub}(unsafe_getindex(ax, idxs)))
+    AxisArray(idxs + idx.index, Axis{:sub}(inbounds_getindex(ax, idxs)))
 end
 axisindexes(::Type{Dimensional}, ax::AbstractVector, idx::RepeatedIntervalAtIndexes) = error("repeated intervals might select a varying number of elements for non-range axes; use a repeated Range of indices instead")
 function axisindexes(::Type{Dimensional}, ax::Range, idx::RepeatedIntervalAtIndexes)
     n = length(idx.indexes)
     idxs = unsafe_searchsorted(ax, idx.window)
-    AxisArray(RepeatedRangeMatrix(idxs, idx.indexes), Axis{:sub}(unsafe_getindex(ax, idxs)), Axis{:rep}(ax[idx.indexes]))
+    AxisArray(RepeatedRangeMatrix(idxs, idx.indexes), Axis{:sub}(inbounds_getindex(ax, idxs)), Axis{:rep}(ax[idx.indexes]))
 end
 
 # Categorical axes may be indexed by their elements

src/search.jl

@@ -37,26 +37,28 @@ function Base.searchsorted(a::Range, I::ClosedInterval)
 end
 
 # When running with "--check-bounds=yes" (like on Travis), the bounds-check isn't elided
-@inline function Base.unsafe_getindex{T}(v::Range{T}, i::Integer)
+@inline function inbounds_getindex{T}(v::Range{T}, i::Integer)
     convert(T, first(v) + (i-1)*step(v))
 end
-@inline function Base.unsafe_getindex{T<:Integer}(r::StepRange, s::Range{T})
-    st = oftype(r.start, r.start + (first(s)-1)*step(r))
+@inline function inbounds_getindex{T<:Integer}(r::Range, s::Range{T})
+    f = first(r)
+    st = oftype(f, f + (first(s)-1)*step(r))
     range(st, step(r)*step(s), length(s))
 end
 if VERSION < v"0.6.0-dev.2390"
     include_string("""
-    @inline function Base.unsafe_getindex{T}(r::FloatRange{T}, i::Integer)
+    @inline function inbounds_getindex{T}(r::FloatRange{T}, i::Integer)
         convert(T, (r.start + (i-1)*r.step)/r.divisor)
     end
-    @inline function Base.unsafe_getindex(r::FloatRange, s::OrdinalRange)
+    @inline function inbounds_getindex(r::FloatRange, s::OrdinalRange)
         FloatRange(r.start + (first(s)-1)*r.step, step(s)*r.step, length(s), r.divisor)
     end
     """)
 else
     include_string("""
-    @inline function Base.unsafe_getindex(r::StepRangeLen, s::OrdinalRange)
-        vfirst = unsafe_getindex(r, first(s))
+    @inline inbounds_getindex(r::StepRangeLen, i::Integer) = Base.unsafe_getindex(r, i)
+    @inline function inbounds_getindex(r::StepRangeLen, s::OrdinalRange)
+        vfirst = Base.unsafe_getindex(r, first(s))
         StepRangeLen(vfirst, r.step*step(s), length(s))
     end
     """)
@@ -65,12 +67,12 @@ end
 function unsafe_searchsortedlast{T<:Number}(a::Range{T}, x::Number)
     step(a) == 0 && throw(ArgumentError("ranges with a zero step are unsupported"))
     n = round(Integer,(x-first(a))/step(a))+1
-    isless(x, unsafe_getindex(a, n)) ? n-1 : n
+    isless(x, inbounds_getindex(a, n)) ? n-1 : n
 end
 function unsafe_searchsortedfirst{T<:Number}(a::Range{T}, x::Number)
     step(a) == 0 && throw(ArgumentError("ranges with a zero step are unsupported"))
     n = round(Integer,(x-first(a))/step(a))+1
-    isless(unsafe_getindex(a, n), x) ? n+1 : n
+    isless(inbounds_getindex(a, n), x) ? n+1 : n
 end
 function unsafe_searchsortedlast{T<:Integer}(a::Range{T}, x::Number)
     step(a) == 0 && throw(ArgumentError("ranges with a zero step are unsupported"))

test/REQUIRE

@@ -1,2 +1,2 @@
 OffsetArrays
-Unitful
+Unitful 0.1.0

test/indexing.jl

@@ -37,6 +37,8 @@ D[1,1,1,1,1] = 10
 
 # Linear indexing across multiple dimensions drops tracking of those dims
 @test A[:].axes[1].val == 1:length(A)
+# TODO: remove the next 4 lines when we no longer feel we need to test for this
+VERSION >= v"0.6.0-dev" && info("partial linear indexing deprecation warning is expected")
 B = A[1:2,:]
 @test B.axes[1].val == A.axes[1].val[1:2]
 @test B.axes[2].val == 1:Base.trailingsize(A,2)