Make iterate(::Reverse) use indexing by default

[MasonProtter]

With this change we can do things like

julia> struct MyVector
           v::Vector
       end

julia> Base.getindex(mv::MyVector, args...) = Base.getindex(mv.v, args...)

julia> Base.eachindex(mv::MyVector) = eachindex(mv.v)

julia> for x ∈ Iterators.reverse(MyVector([:a, :b, :c]))
           @show x
       end
x = :c
x = :b
x = :a

I'm proposing this as an alternative to #43106 in order to fix #43101

[MasonProtter]

I think the most significant downside I can forsee of this is that if one has a type that does not support indexing, then you may get worse error messages than before.

Compare (old)

julia> struct Foo
           a::Int
           b::Int
       end

julia> for x ∈ Iterators.reverse(Foo(1, 2))
           @show x
       end
ERROR: MethodError: no method matching iterate(::Base.Iterators.Reverse{Foo})
Closest candidates are:
  iterate(::Union{LinRange, StepRangeLen}) at ~/julia/usr/share/julia/base/range.jl:821
  iterate(::Union{LinRange, StepRangeLen}, ::Integer) at ~/julia/usr/share/julia/base/range.jl:821
  iterate(::T) where T<:Union{Base.KeySet{<:Any, <:Dict}, Base.ValueIterator{<:Dict}} at ~/julia/usr/share/julia/base/dict.jl:695
  ...
Stacktrace:
 [1] top-level scope
   @ ./REPL[2]:1

against (new)

julia> struct Foo
           a::Int
           b::Int
       end

julia> for x ∈ Iterators.reverse(Foo(1, 2))
           @show x
       end
ERROR: MethodError: no method matching keys(::Foo)
Closest candidates are:
  keys(::IndexStyle, ::AbstractArray, ::AbstractArray...) at ~/julia/usr/share/julia/base/abstractarray.jl:350
  keys(::Tuple) at ~/julia/usr/share/julia/base/tuple.jl:72
  keys(::Tuple, ::Tuple...) at ~/julia/usr/share/julia/base/tuple.jl:77
  ...
Stacktrace:
 [1] eachindex(itrs::Foo)
   @ Base ./abstractarray.jl:276
 [2] iterate(A::Base.Iterators.Reverse{Foo})
   @ Base.Iterators ./REPL[1]:2
 [3] top-level scope
   @ ./REPL[7]:1

[vtjnash]

I think we should do this in addition to #43106

[Seelengrab]

Since Iterators.reverse is a lazy wrapper anyway, we could check isapplicable on construction and manually throw a MethodError, right? Wouldn't be pretty, but the construction itself is probably not performance critical and it wouldn't be the first time a custom MethodError is thrown for clarity sake.

[JeffBezanson]

Suggested change

	function iterate(A::Reverse state=(reverse(eachindex(A.itr)),))
	function iterate(A::Reverse, state=(reverse(eachindex(A.itr)),))

I guess this could lead to a dispatch loop if an array's index iterator doesn't support reverse? Maybe ok though.

[MasonProtter]

Maybe use Base.reverse?

[MasonProtter]

I think we should do this in addition to #43106

But #43106 makes last use indexing instead of reverse by default..

[JeffBezanson]

Our thinking is that implementing last is easier (and could be implemented for strictly more types) than implementing reverse iteration, so we might as well have both fall back to indexing, and types with reverse iteration should just add last methods.