Make view of range with range be a range (#26872)

* Make view of range with range be a range

Typical `AbstractRange` types are compact and immutable, and we can
remove the overhead of `SubArray` in this case while the cost of `view`
remains O(1).

Here we specialize only on valid combinations of integer ranges,
otherwise still relying on `SubArray`.

* Cover all implementations of `getindex(::AbstractRange,...)`

* Add NEWS

Co-authored-by: Andy Ferris <[email protected]>
Co-authored-by: Matt Bauman <[email protected]>

Author: 3 people

NEWS.md

@@ -143,6 +143,8 @@ Standard library changes
 * A 1-d `Zip` iterator (where `Base.IteratorSize` is `Base.HasShape{1}()`) with defined length of `n` has now also size of `(n,)` (instead of throwing an error with truncated iterators) ([#29927]).
 * The `@timed` macro now returns a `NamedTuple` ([#34149])
 * New `supertypes(T)` function returns a tuple of all supertypes of `T` ([#34419]).
+* Views of builtin ranges are now recomputed ranges (like indexing returns) instead of
+  `SubArray`s ([#26872]).
 * Sorting-related functions such as `sort` that take the keyword arguments `lt`, `rev`, `order`
   and `by` now do not discard `order` if `by` or `lt` are passed. In the former case, the
   order from `order` is used to compare the values of `by(element)`. In the latter case,

base/subarray.jl

@@ -164,6 +164,32 @@ function view(A::AbstractArray, I::Vararg{Any,N}) where {N}
     unsafe_view(_maybe_reshape_parent(A, index_ndims(J...)), J...)
 end
 
+# Ranges implement getindex to return recomputed ranges; use that for views, too (when possible)
+function view(r1::OneTo, r2::OneTo)
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+function view(r1::AbstractUnitRange, r2::AbstractUnitRange{<:Integer})
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+function view(r1::AbstractUnitRange, r2::StepRange{<:Integer})
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+function view(r1::StepRange, r2::AbstractRange{<:Integer})
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+function view(r1::StepRangeLen, r2::OrdinalRange{<:Integer})
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+function view(r1::LinRange, r2::OrdinalRange{<:Integer})
+    @_propagate_inbounds_meta
+    getindex(r1, r2)
+end
+
 function unsafe_view(A::AbstractArray, I::Vararg{ViewIndex,N}) where {N}
     @_inline_meta
     SubArray(A, I)

test/ranges.jl

@@ -1401,6 +1401,22 @@ end
     @test x isa StepRangeLen{Float64,Base.TwicePrecision{Float64},Base.TwicePrecision{Float64}}
 end
 
+@testset "Views of ranges" begin
+    @test view(Base.OneTo(10), Base.OneTo(5)) === Base.OneTo(5)
+    @test view(1:10, 1:5) === 1:5
+    @test view(1:10, 1:2:5) === 1:2:5
+    @test view(1:2:9, 1:5) === 1:2:9
+
+    # Ensure we don't hit a fallback `view` if there's a better `getindex` implementation
+    vmt = collect(methods(view, Tuple{AbstractRange, AbstractRange}))
+    for m in methods(getindex, Tuple{AbstractRange, AbstractRange})
+        tt = Base.tuple_type_tail(m.sig)
+        tt == Tuple{AbstractArray,Vararg{Any,N}} where N && continue
+        vm = findfirst(sig->tt <: Base.tuple_type_tail(sig.sig), vmt)
+        @test vmt[vm].sig != Tuple{typeof(view),AbstractArray,Vararg{Any,N}} where N
+    end
+end
+
 @testset "Issue #26608" begin
     @test_throws BoundsError (Int8(-100):Int8(100))[400]
     @test_throws BoundsError (-100:100)[typemax(UInt)]