JuliaLang / julia / #38131

77%

Build Type

push

local

Committed by web-flow

Commit Message

Fix unique for range wrappers with zero step (#51004)

The current implementation assumes that the vector indexing
`r[begin:begin]` is specialized to return a range, which isn't the case
by default. As a consequence,
```julia
julia> struct MyStepRangeLen{T,R} <: AbstractRange{T}
           x :: R
       end

julia> MyStepRangeLen(s::StepRangeLen{T}) where {T} = MyStepRangeLen{T,typeof(s)}(s)
MyStepRangeLen

julia> Base.first(s::MyStepRangeLen) = first(s.x)

julia> Base.last(s::MyStepRangeLen) = last(s.x)

julia> Base.length(s::MyStepRangeLen) = length(s.x)

julia> Base.step(s::MyStepRangeLen) = step(s.x)

julia> r = MyStepRangeLen(StepRangeLen(1,0,4))
1:0:1

julia> unique(r)
ERROR: MethodError: Cannot `convert` an object of type Vector{Int64} to an object of type MyStepRangeLen{Int64, Int64, StepRangeLen{Int64, Int64, Int64, Int64}}
[...]
```
This PR fixes this by using constructing a `UnitRange` instead of the
indexing operation. After this, we obtain
```julia
julia> unique(r)
1:1:1
```
In principle, the `step` should be preserved, but `range(r[begin]::Int,
step=step(r), length=length(r))` appears to error at present, as it
tries to construct a `StepRange` instead of a `StepRangeLen`.

This fix isn't perfect as it assumes that the conversion from a
`UnitRange` _is_ defined, which is also not the case by default. For
example, the following still won't work:
```julia
julia> struct MyRange <: AbstractRange{Int} end

julia> Base.first(x::MyRange) = 1

julia> Base.last(x::MyRange) = 1

julia> Base.length(x::MyRange) = 3

julia> Base.step(x::MyRange) = 0

julia> unique(MyRange())
ERROR: MethodError: no method matching MyRange(::UnitRange{Int64})
[...]
```
In fact, if the indexing `MyRange()[begin:begin]` has been specialized
but the conversion from a `UnitRange` isn't, then this is actually a
regression. I'm unsure if such pathological cases are common, though.
The reason the first example works is that the conversion for a range
wrapper is defined implici... (continued)

Run Details

0 of 1 new or added line in 1 file covered. (0.0%)

142 existing lines in 2 files now uncovered.

12877 of 50147 relevant lines covered (25.68%)

734624.21 hits per line