Should we disallow non-compliant `AbstractUnitRange`s where `typeof(step)` doesn't match the `eltype`?

Currently, the docstring for AbstractUnitRange states:

    AbstractUnitRange{T} <: OrdinalRange{T, T}

Supertype for ranges with a step size of `oneunit(T)` with elements of type `T`.

This implies that the step must be of type T as well. However, this is not enforced anywhere, and as a consequence, one may create ranges for which this is not obeyed at all. Even the test suite for ranges in Base has some examples like this:

struct Position <: Integer
    val::Int
end
Position(x::Position) = x # to resolve ambiguity with boot.jl:770

struct Displacement <: Integer
    val::Int
end
Displacement(x::Displacement) = x # to resolve ambiguity with boot.jl:770

Base.:-(x::Displacement) = Displacement(-x.val)
Base.:-(x::Position, y::Position) = Displacement(x.val - y.val)
Base.:-(x::Position, y::Displacement) = Position(x.val - y.val)
Base.:-(x::Displacement, y::Displacement) = Displacement(x.val - y.val)
Base.:+(x::Position, y::Displacement) = Position(x.val + y.val)
Base.:+(x::Displacement, y::Displacement) = Displacement(x.val + y.val)
Base.:(<=)(x::Position, y::Position) = x.val <= y.val
Base.:(<)(x::Position, y::Position) = x.val < y.val
Base.:(<)(x::Displacement, y::Displacement) = x.val < y.val

# for StepRange computation:
Base.Unsigned(x::Displacement) = Unsigned(x.val)
Base.rem(x::Displacement, y::Displacement) = Displacement(rem(x.val, y.val))
Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val))

# required for collect (summing lengths); alternatively, should length return Int by default?
Base.promote_rule(::Type{Displacement}, ::Type{Int}) = Int
Base.convert(::Type{Int}, x::Displacement) = x.val
Base.Int(x::Displacement) = x.val

With this definition, one may define:

julia> r = Position(2):Position(5)
Position(2):Position(5)

julia> r isa AbstractUnitRange{Position}
true

julia> step(r)
Displacement(1)

julia> step(r) |> typeof
Displacement

This range is, strictly speaking, a StepRange, with a unit step. We don’t have a UnitStepRange type in Base, which leads to the abuse of UnitRanges. However, disregarding type-parameters in this manner has the potential to open up unexpected bugs. Is it reasonable to disallow such non-compliant AbstractUnitRanges? I understand that there are concerns about the performances of StepRanges, but perhaps we may add a UnitStepRange type to get around these?

As an example, Dates already avoids this issue in the colon range constructor, and returns a StepRange instead:

julia> today():today()
Date("2023-07-17"):Day(1):Date("2023-07-17")

The docstring for Colon states that it produces

 a `UnitRange` when a and b are integers,

but that may be altered. IMO this should not produce a UnitRange if typeof(a-b) != typeof(a)

2 Likes

I don’t think it needs to be altered; Day(1) is not an integer.

It’s fine to have step be different than the eltype; it’s only required to be of the same type as the additive element of the eltype. It’s similar to how you use 'a' + 1 to get 'b' and not 'a' + Char(0x1).

2 Likes

The definition AbstractUnitRange{T} <: OrdinalRange{T, T} requires step to be of the same type.

I don’t think it needs to be altered; Day(1) is not an integer.

The integer example is the Position range from the Base test suite. The point actually goes the other way. It’s fine for any type to return a StepRange instead of a UnitRange, and the Day example was that of a type that chooses the range type correctly.

1 Like

That’s a good point, and a bit bothersome :thinking: We don’t have a oneadditiveunit equivalent, unfortunately…

2 Likes

To be more concrete about the issue in the OP: in the Position example, one may add a Position and a Displacement to obtain another Position, but there’s no notion of adding a Position to another Position, or to an Int. As a consequence, indexing is broken for a UnitRange{Position}:

julia> r = Position(2):Position(5)
Position(2):Position(5)

julia> r[2]
ERROR: promotion of types Position and Int64 failed to change any arguments

whereas, for a StepRange, if we define a few missing methods:

julia> Base.:(*)(i::Int, x::Displacement) = Displacement(x.val * i)

julia> Base.:(*)(x::Displacement, i::Int) = Displacement(x.val * i)

we may obtain

julia> s = Position(2):Displacement(1):Position(5)
Position(2):Displacement(1):Position(5)

julia> s[2]
Position(3)

where we are adding a Displacement to a Position.

1 Like

Actually, Position is the odd one out here, because it subtypes Integer. Since 'a':'z' is conceptually the same thing (the additive element is different from the actual type) and that already gives a StepRange, looking at @edit 'a':'z' clears things up a bit:

(:)(a::Real, b::Real) = (:)(promote(a, b)...)

(:)(start::T, stop::T) where {T<:Real} = UnitRange{T}(start, stop)

(:)(start::T, stop::T) where {T} = (:)(start, oftype(stop >= start ? stop - start : start - stop, 1), stop)

So : actually only assumes that step is a T for Reals, but for all others, it goes through the same oftype construction as Char goes through. Dates just overloads that conceptual fallback in a way that doesn’t rely on convert(Day, ::Int) to exist.

I guess I agree then - the docstring for : ought to be changed to "with a step size equal to oftype(..., 1) (not quite sure what to put in there).

Then again - perhaps this is just another example of underspecified requirements of functions from Base…

2 Likes

I’d say it should be allowed because differences of quantities like dates can’t be treated the same as the absolute values, I’m sure there’s a formal math term for numbers like that. Problem is I’m not sure how to patch this without changing the type parameters to AbstractUnitRange{T, S}, which seems breaking. It’d be nice to have AbstractUnitRange{T} <: OrdinalRange{T, S} where S is promised to have a 1-to-1 correspondence with T in AbstractUnitRanges.

1 Like

But probably should be

'a' + NextCharacterInTermsOfUnicodeCodepoints(1)

1 Like