Bug in @view: begin indexing vs. replace_ref_begin_end!

[phipsgabler]

I thought Base.replace_ref_begin_end! is used to implement begin indexing, but appearently they are inconsistent?

Reading the source of it, I stumbled upon this line in views.jl:

exj, used = replace_ref_begin_end_!(ex.args[j], (:($firstindex($S)),:($lastindex($S,$n))))

Shouldn't that be $firstindex($S, $n)? I tried producing a failing example using OffsetArrays, but that only showed some sort of discrepancy, which I can't explain:

julia> a
2×2 OffsetArray(::Array{Int64,2}, -10:-9, 9:10) with eltype Int64 with indices -10:-9×9:10:
 1  2
 3  4

julia> a[end, begin]
3

julia> Meta.@lower a[end, begin]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.lastindex(a, 1)
│   %2 = Base.axes(a, 2)
│   %3 = Base.first(%2)
│   %4 = Base.getindex(a, %1, %3)
└──      return %4
))))

julia> Base.replace_ref_begin_end!(:(a[end, begin]))
:(a[(lastindex)(a, 1), (firstindex)(a)])

julia> a[(lastindex)(a, 1), (firstindex)(a)]
ERROR: BoundsError: attempt to access 2×2 OffsetArray(::Array{Int64,2}, -10:-9, 9:10) with eltype Int64 with indices -10:-9×9:10 at index [-9, 1]
Stacktrace:
 [1] throw_boundserror(::OffsetArray{Int64,2,Array{Int64,2}}, ::Tuple{Int64,Int64}) at ./abstractarray.jl:541
 [2] checkbounds at ./abstractarray.jl:506 [inlined]
 [3] getindex(::OffsetArray{Int64,2,Array{Int64,2}}, ::Int64, ::Int64) at /home/philipp/.julia/packages/OffsetArrays/Osysv/src/OffsetArrays.jl:407
 [4] top-level scope at REPL[22]:1

[phipsgabler]

Now watch this:

julia> a = OffsetArray([1 2; 3 4], -10:-9, 9:10)
2×2 OffsetArray(::Array{Int64,2}, -10:-9, 9:10) with eltype Int64 with indices -10:-9×9:10:
 1  2
 3  4

julia> @view a[end, begin]
ERROR: BoundsError: attempt to access 2×2 OffsetArray(::Array{Int64,2}, -10:-9, 9:10) with eltype Int64 with indices -10:-9×9:10 at index [-9, 1]
Stacktrace:
 [1] throw_boundserror(::OffsetArray{Int64,2,Array{Int64,2}}, ::Tuple{Int64,Int64}) at ./abstractarray.jl:541
 [2] checkbounds at ./abstractarray.jl:506 [inlined]
 [3] view(::OffsetArray{Int64,2,Array{Int64,2}}, ::Int64, ::Int64) at ./subarray.jl:158
 [4] top-level scope at REPL[5]:1

[simeonschaub]

Yeah, that's definitely a bug. Should be easy enough to fix though.

[phipsgabler]

Fixing the one line in replace_ref_begin_end! is easy, sure, and I can come up with a PR for it.

But is there any chance to reuse whatever is happening internally, so that macro writers can rely on matching behaviour? This:

julia> Meta.@lower a[end, begin]
:($(Expr(:thunk, CodeInfo(
    @ none within `top-level scope'
1 ─ %1 = Base.lastindex(a, 1)
│   %2 = Base.axes(a, 2)
│   %3 = Base.first(%2)
│   %4 = Base.getindex(a, %1, %3)
└──      return %4
))))

looks like it has never seen replace_ref_begin_end!. It uses axes, not firstindex. Or am I mistaken?

[simeonschaub]

The way our frontend is currently implemented, there's not really a nice way to do this. It is possible that might get rewritten in Julia some day and we might be able to expose some of it to macros, but there aren't any plans for that currently.