Skip to content

Commit 6ddb3d6

Browse files
authored
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 implicitly if the parent type supports conversion from a `UnitRange`.
1 parent 01b1348 commit 6ddb3d6

File tree

2 files changed

+13
-1
lines changed

2 files changed

+13
-1
lines changed

base/set.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ _unique_from(itr, out, seen, i) = unique_from(itr, out, seen, i)
259259
return out
260260
end
261261

262-
unique(r::AbstractRange) = allunique(r) ? r : oftype(r, r[begin:begin])
262+
unique(r::AbstractRange) = allunique(r) ? r : oftype(r, r[begin]:r[begin])
263263

264264
"""
265265
unique(f, itr)

test/ranges.jl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -699,6 +699,18 @@ end
699699
@test Duck(4) Duck(1):Duck(5)
700700
@test Duck(0) Duck(1):Duck(5)
701701
end
702+
@testset "unique" begin
703+
struct MyStepRangeLen{T,R} <: AbstractRange{T}
704+
x :: R
705+
end
706+
MyStepRangeLen(s::StepRangeLen{T}) where {T} = MyStepRangeLen{T,typeof(s)}(s)
707+
Base.first(s::MyStepRangeLen) = first(s.x)
708+
Base.last(s::MyStepRangeLen) = last(s.x)
709+
Base.length(s::MyStepRangeLen) = length(s.x)
710+
Base.step(s::MyStepRangeLen) = step(s.x)
711+
sr = StepRangeLen(1,0,4)
712+
@test unique(MyStepRangeLen(sr)) == unique(sr)
713+
end
702714
end
703715
@testset "indexing range with empty range (#4309)" begin
704716
@test (@inferred (3:6)[5:4]) === 7:6

0 commit comments

Comments
 (0)