Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow ranges with Integer eltype but non-integer step #32439

Merged
merged 14 commits into from
Nov 30, 2020
Merged
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ New library features
Standard library changes
------------------------
* A 1-d `Zip` iterator (where `Base.IteratorSize` is `Base.HasShape{1}()`) with defined length of `n` has now also size of `(n,)` (instead of throwing an error with truncated iterators) ([#29927]).
* It is no longer possible to create a `LinRange`, `StepRange`, or `StepRangeLen` with a
`<: Integer` eltype but non-integer step ([#32439]).
* The `@timed` macro now returns a `NamedTuple` ([#34149])
* New `supertypes(T)` function returns a tuple of all supertypes of `T` ([#34419]).
* Sorting-related functions such as `sort` that take the keyword arguments `lt`, `rev`, `order`
Expand Down
12 changes: 11 additions & 1 deletion base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ function steprange_last(start::T, step, stop) where T
if isa(start,AbstractFloat) || isa(step,AbstractFloat)
throw(ArgumentError("StepRange should not be used with floating point"))
end
if isa(start,Integer) && !isinteger(step)
throw(ArgumentError("StepRange{<:Integer} cannot have non-integer step"))
end
z = zero(step)
step == z && throw(ArgumentError("step cannot be zero"))

Expand Down Expand Up @@ -344,6 +347,9 @@ struct StepRangeLen{T,R,S} <: AbstractRange{T}
offset::Int # the index of ref

function StepRangeLen{T,R,S}(ref::R, step::S, len::Integer, offset::Integer = 1) where {T,R,S}
if T <: Integer && !isinteger(step)
throw(ArgumentError("StepRangeLen{<:Integer} cannot have non-integer step"))
end
len >= 0 || throw(ArgumentError("length cannot be negative, got $len"))
1 <= offset <= max(1,len) || throw(ArgumentError("StepRangeLen: offset must be in [1,$len], got $offset"))
new(ref, step, len, offset)
Expand Down Expand Up @@ -403,7 +409,11 @@ struct LinRange{T} <: AbstractRange{T}
start == stop || throw(ArgumentError("range($start, stop=$stop, length=$len): endpoints differ"))
return new(start, stop, 1, 1)
end
new(start,stop,len,max(len-1,1))
lendiv = max(len-1, 1)
if T <: Integer && !iszero(mod(stop-start, lendiv))
throw(ArgumentError("LinRange{<:Integer} cannot have non-integer step"))
end
new(start,stop,len,lendiv)
end
end

Expand Down
10 changes: 10 additions & 0 deletions test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -997,6 +997,15 @@ end
@test eltype(['a':'z', 1:2]) == (StepRange{T,Int} where T)
end

@testset "Ranges with <:Integer eltype but non-integer step (issue #32419)" begin
@test eltype(StepRange(1, 1//1, 2)) <: Integer
@test_throws ArgumentError StepRange(1, 1//2, 2)
@test eltype(StepRangeLen{Int}(1, 1//1, 2)) <: Integer
@test_throws ArgumentError StepRangeLen{Int}(1, 1//2, 2)
@test eltype(LinRange{Int}(1, 5, 3)) <: Integer
@test_throws ArgumentError LinRange{Int}(1, 5, 4)
end

@testset "LinRange ops" begin
@test 2*LinRange(0,3,4) == LinRange(0,6,4)
@test LinRange(0,3,4)*2 == LinRange(0,6,4)
Expand Down Expand Up @@ -1324,6 +1333,7 @@ Base.:+(x, y::NotReal) = x + y.val
Base.zero(y::NotReal) = zero(y.val)
Base.rem(x, y::NotReal) = rem(x, y.val)
Base.isless(x, y::NotReal) = isless(x, y.val)
Base.isinteger(y::NotReal) = isinteger(y.val)
sostock marked this conversation as resolved.
Show resolved Hide resolved
@test (:)(1, NotReal(1), 5) isa StepRange{Int,NotReal}

isdefined(Main, :Furlongs) || @eval Main include("testhelpers/Furlongs.jl")
Expand Down