Skip to content

Commit

Permalink
fix method discrepencies
Browse files Browse the repository at this point in the history
  • Loading branch information
vtjnash committed Jun 9, 2021
1 parent 1424822 commit 96a1e5f
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 46 deletions.
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ Standard library changes
* `replace(::String)` now accepts multiple patterns, which will be applied left-to-right simultaneously,
so only one pattern will be applied to any character, and the patterns will only be applied to the input
text, not the replacements ([#40484]).
* The `length` function on certain ranges of certain specific element types no longer checks for integer
overflow in most cases. The new function `checked_length` is now available, which will try to use checked
arithmetic to error if the result may be wrapping. Or use a package such as SaferIntegers.jl when
constructing the range. ([#40382])

#### Package Manager

Expand Down
2 changes: 1 addition & 1 deletion base/checked.jl
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ checked_cld(x::T, y::T) where {T<:Integer} = cld(x, y) # Base.cld already checks
Base.checked_length(r)
Calculates `length(r)`, but may check for overflow errors where applicable when
the result doesn't fit into `Union{eltype(r),Int}`.
the result doesn't fit into `Union{Integer(eltype(r)),Int}`.
"""
checked_length(r) = length(r) # for most things, length doesn't error

Expand Down
32 changes: 26 additions & 6 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -640,7 +640,15 @@ function checked_length(r::OrdinalRange{T}) where T
if s == zero(s) || isempty(r)
return Integer(start - start + zero(s))
end
return Integer(div(checked_add(checked_sub(last(r), start), s, s)))
stop = last(r)
if isless(s, zero(s))
diff = checked_sub(start, stop)
s = -s
else
diff = checked_sub(stop, start)
end
a = Integer(div(diff, s))
return checked_add(a, one(a))
end

function checked_length(r::AbstractUnitRange{T}) where T
Expand All @@ -649,7 +657,7 @@ function checked_length(r::AbstractUnitRange{T}) where T
return Integer(first(r) - first(r))
end
a = Integer(checked_add(checked_sub(last(r), first(r))))
return a + one(a)
return checked_add(a, one(a))
end

function length(r::OrdinalRange{T}) where T
Expand All @@ -659,9 +667,18 @@ function length(r::OrdinalRange{T}) where T
if s == zero(s) || isempty(r)
return Integer(start - start + zero(s))
end
return Integer(div(last(r) - start + s, s))
stop = last(r)
if isless(s, zero(s))
diff = start - stop
s = -s
else
diff = stop - start
end
a = Integer(div(diff, s))
return a + one(a)
end


function length(r::AbstractUnitRange{T}) where T
@_inline_meta
a = Integer(last(r) - first(r)) # even when isempty, by construction (with overflow)
Expand All @@ -673,7 +690,7 @@ length(r::StepRangeLen) = r.len
length(r::LinRange) = r.len

let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128}
global length
global length, checked_length
# compile optimization for which promote_type(T, Int) == T
length(r::OneTo{T}) where {T<:bigints} = r.stop
# slightly more accurate length and checked_length in extreme cases
Expand Down Expand Up @@ -720,11 +737,14 @@ end
let smallints = (Int === Int64 ?
Union{Int8, UInt8, Int16, UInt16, Int32, UInt32} :
Union{Int8, UInt8, Int16, UInt16})
global length
global length, checked_length
# n.b. !(step isa T)
length(r::OrdinalRange{<:smallints}) = div(Int(last(r)) - Int(first(r)) + step(r), step(r))
length(r::OrdinalRange{<:smallints}) = div(Int(last(r)) - Int(first(r)), step(r)) + 1
length(r::AbstractUnitRange{<:smallints}) = Int(last(r)) - Int(first(r)) + 1
length(r::OneTo{<:smallints}) = Int(r.stop)
checked_length(r::OrdinalRange{<:smallints}) = length(r)
checked_length(r::AbstractUnitRange{<:smallints}) = length(r)
checked_length(r::OneTo{<:smallints}) = length(r)
end

first(r::OrdinalRange{T}) where {T} = convert(T, r.start)
Expand Down
1 change: 1 addition & 0 deletions doc/src/base/collections.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ Base.LinRange
Base.isempty
Base.empty!
Base.length
Base.checked_length
```

Fully implemented by:
Expand Down
165 changes: 126 additions & 39 deletions test/ranges.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license

using Base.Checked: checked_length

@testset "range construction" begin
@test_throws ArgumentError range(start=1, step=1, stop=2, length=10)
@test_throws ArgumentError range(start=1, step=1, stop=10, length=11)
Expand Down Expand Up @@ -267,22 +269,28 @@ end
end
end
@testset "length" begin
@test length(.1:.1:.3) == 3
@test length(1.1:1.1:3.3) == 3
@test length(1.1:1.3:3) == 2
@test length(1:1:1.8) == 1
@test length(1:.2:2) == 6
@test length(1.:.2:2.) == 6
@test length(2:-.2:1) == 6
@test length(2.:-.2:1.) == 6
@test length(2:.2:1) == 0
@test length(.1:.1:.3) == checked_length(.1:.1:.3) == 3
@test length(1.1:1.1:3.3) == checked_length(1.1:1.1:3.3) == 3
@test length(1.1:1.3:3) == checked_length(1.1:1.3:3) == 2
@test length(1:1:1.8) == checked_length(1:1:1.8) == 1
@test length(1:.2:2) == checked_length(1:.2:2) == 6
@test length(1.:.2:2.) == checked_length(1.:.2:2.) == 6
@test length(2:-.2:1) == checked_length(2:-.2:1) == 6
@test length(2.:-.2:1.) == checked_length(2.:-.2:1.) == 6
@test length(2:.2:1) == checked_length(2:.2:1) == 0
@test length(2.:.2:1.) == 0

@test length(1:0) == 0
@test length(0.0:-0.5) == 0
@test length(1:2:0) == 0
@test length(Char(0):Char(0x001fffff)) == 2097152
@test length(typemax(UInt64)//one(UInt64):1:typemax(UInt64)//one(UInt64)) == 1
@test length(1:0) == checked_length(1:0) == 0
@test length(0.0:-0.5) == checked_length(0.0:-0.5) == 0
@test length(1:2:0) == checked_length(1:2:0) == 0
let r = Char(0):Char(0x001fffff)
@test length(r) == 2097152
@test_throws MethodError checked_length(r) == 2097152 # this would work if checked_sub is defined on Char
end
let r = typemax(UInt64)//one(UInt64):1:typemax(UInt64)//one(UInt64)
@test length(r) == 1
@test_throws MethodError checked_length(r) == 1 # this would work if checked_sub is defined on Rational
end
end
@testset "keys/values" begin
keytype_is_correct(r) = keytype(r) == eltype(keys(r))
Expand Down Expand Up @@ -496,7 +504,8 @@ for a=AbstractRange[3:6, 0:2:10], b=AbstractRange[0:1, 2:-1:0]
end

# avoiding intermediate overflow (#5065)
@test length(1:4:typemax(Int)) == div(typemax(Int),4) + 1
@test length(1:4:typemax(Int)) == div(typemax(Int), 4) + 1
@test checked_length(1:4:typemax(Int)) == div(typemax(Int), 4) + 1 # computed exactly in modulo arithmetic

@testset "overflow in length" begin
Tset = Int === Int64 ? (Int, UInt, Int128, UInt128) :
Expand All @@ -506,7 +515,11 @@ end
@test length(typemin(T):typemax(T)) == T(0)
@test length(zero(T):one(T):typemax(T)) == typemin(T)
@test length(typemin(T):one(T):typemax(T)) == T(0)
@test length(one(T):typemax(T)) == typemax(T)
@test_throws OverflowError checked_length(zero(T):typemax(T))
@test_throws OverflowError checked_length(typemin(T):typemax(T))
@test_throws OverflowError checked_length(zero(T):one(T):typemax(T))
@test_throws OverflowError checked_length(typemin(T):one(T):typemax(T))
@test length(one(T):typemax(T)) == checked_length(one(T):typemax(T)) == typemax(T)
if T <: Signed
@test length(-one(T):typemax(T)-one(T)) == typemin(T)
@test length(-one(T):one(T):typemax(T)-one(T)) == typemin(T)
Expand All @@ -517,6 +530,11 @@ end
@test length(-one(T):typemin(T):typemin(T)) == 1
@test length(zero(T):typemin(T):zero(T)) == 1
@test length(zero(T):typemin(T):one(T)) == 0
@test_throws OverflowError checked_length(-one(T):typemax(T)-one(T))
@test_throws OverflowError checked_length(-one(T):one(T):typemax(T)-one(T))
@test_throws InexactError checked_length(zero(T):typemin(T):typemin(T)) == 2 # this can be improved
@test_throws InexactError checked_length(one(T):typemin(T):typemin(T)) == 2 # this can be improved
@test_throws InexactError checked_length(typemax(T):typemin(T):typemin(T)) == 2 # this can be improved
end
end
end
Expand Down Expand Up @@ -882,25 +900,36 @@ end
@test first(r) == typemin(Int64)
@test last(r) == typemax(Int64) - 1
@test length(r) == typemin(Int64)
@test_throws OverflowError checked_length(r)
end
let s = typemax(Int64):-2:typemin(Int64)
@test first(s) == typemax(Int64)
@test last(s) == typemin(Int64) + 1
@test length(s) == typemin(Int64)
let r = typemax(Int64):-2:typemin(Int64)
@test first(r) == typemax(Int64)
@test last(r) == typemin(Int64) + 1
@test length(r) == typemin(Int64)
@test_throws OverflowError checked_length(r)
end

@test length(typemin(Int64):3:typemax(Int64)) == 6148914691236517206
@test length(typemax(Int64):-3:typemin(Int64)) == 6148914691236517206
let r = typemin(Int64):3:typemax(Int64)
@test length(r) == checked_length(r) == 6148914691236517206
end
let r = typemax(Int64):-3:typemin(Int64)
@test length(r) == checked_length(r) == 6148914691236517206
end

for s in 3:100
@test length(typemin(Int):s:typemax(Int)) == length(big(typemin(Int)):big(s):big(typemax(Int)))
@test length(typemax(Int):-s:typemin(Int)) == length(big(typemax(Int)):big(-s):big(typemin(Int)))
r = typemin(Int):s:typemax(Int)
br = big(typemin(Int)):big(s):big(typemax(Int))
@test length(r) == checked_length(r) == length(br)

r = typemax(Int):-s:typemin(Int)
br = big(typemax(Int)):big(-s):big(typemin(Int))
@test length(r) == checked_length(r) == length(br)
end

@test length(UInt(1):UInt(1):UInt(0)) == 0
@test length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == 0
@test length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == 0
@test length((typemin(Int)+3):5:(typemin(Int)+1)) == 0
@test length(UInt(1):UInt(1):UInt(0)) == checked_length(UInt(1):UInt(1):UInt(0)) == 0
@test length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == checked_length(typemax(UInt):UInt(1):(typemax(UInt)-1)) == 0
@test length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == checked_length(typemax(UInt):UInt(2):(typemax(UInt)-1)) == 0
@test length((typemin(Int)+3):5:(typemin(Int)+1)) == checked_length((typemin(Int)+3):5:(typemin(Int)+1)) == 0
end

# issue #6364
Expand Down Expand Up @@ -972,15 +1001,16 @@ end
(Int8,UInt8,Int16,UInt16,Int32,UInt32) :
(Int8,UInt8,Int16,UInt16))
for T in smallint
@test length(typemin(T):typemax(T)) == 2^(8*sizeof(T))
s = typemin(T):typemax(T)
@test length(s) == checked_length(s) == 2^(8*sizeof(T))
end
end

# issue #8584
@test (0:1//2:2)[1:2:3] == 0:1//1:1

# issue #12278
@test length(1:UInt(0)) == 0
@test length(1:UInt(0)) == checked_length(1:UInt(0)) == 0

@testset "zip" begin
i = 0
Expand Down Expand Up @@ -1060,6 +1090,7 @@ end
@testset "issue 10950" begin
r = 1//2:3
@test length(r) == 3
@test_throws MethodError checked_length(r) == 3 # this would work if checked_sub is defined on Rational
i = 1
for x in r
@test x == i//2
Expand Down Expand Up @@ -1280,12 +1311,12 @@ end
@testset "OneTo" begin
let r = Base.OneTo(-5)
@test isempty(r)
@test length(r) == 0
@test length(r) == checked_length(r) == 0
@test size(r) == (0,)
end
let r = Base.OneTo(3)
@test !isempty(r)
@test length(r) == 3
@test length(r) == checked_length(r) == 3
@test size(r) == (3,)
@test step(r) == 1
@test first(r) == 1
Expand Down Expand Up @@ -1382,7 +1413,7 @@ end

@testset "issue #20520" begin
r = range(1.3173739f0, stop=1.3173739f0, length=3)
@test length(r) == 3
@test length(r) == checked_length(r) == 3
@test first(r) === 1.3173739f0
@test last(r) === 1.3173739f0
@test r[2] === 1.3173739f0
Expand All @@ -1406,7 +1437,7 @@ using .Main.Furlongs

@testset "dimensional correctness" begin
@test length(Vector(Furlong(2):Furlong(10))) == 9
@test length(range(Furlong(2), length=9)) == 9
@test length(range(Furlong(2), length=9)) == checked_length(range(Furlong(2), length=9)) == 9
@test Vector(Furlong(2):Furlong(1):Furlong(10)) == Vector(range(Furlong(2), step=Furlong(1), length=9)) == Furlong.(2:10)
@test Vector(Furlong(1.0):Furlong(0.5):Furlong(10.0)) ==
Vector(Furlong(1):Furlong(0.5):Furlong(10)) == Furlong.(1:0.5:10)
Expand Down Expand Up @@ -1501,15 +1532,18 @@ module NonStandardIntegerRangeTest

using Test

using Base.Checked: checked_length
import Base.Checked: checked_add, checked_sub

struct Position <: Integer
val::Int
end
Position(x::Position) = x # to resolve ambiguity with boot.jl:728
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:728
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)
Expand All @@ -1530,10 +1564,63 @@ Base.div(x::Displacement, y::Displacement) = Displacement(div(x.val, y.val))
Base.promote_rule(::Type{Displacement}, ::Type{Int}) = Int
Base.convert(::Type{Int}, x::Displacement) = x.val

# Unsigned complement, for testing checked_length
struct UPosition <: Unsigned
val::UInt
end
UPosition(x::UPosition) = x # to resolve ambiguity with boot.jl:770

struct UDisplacement <: Unsigned
val::UInt
end
UDisplacement(x::UDisplacement) = x # to resolve ambiguity with boot.jl:770

Base.show(io::IO, x::Union{Position, UPosition, Displacement, UDisplacement}) =
# should use show if we were to do this properly (instead of just a test-helper)
print(io, typeof(x).name.name, "(", x.val, ")")

Base.:-(x::UPosition, y::UPosition) = UDisplacement(x.val - y.val)
Base.:-(x::UPosition, y::UDisplacement) = UPosition(x.val - y.val)
Base.:+(x::UPosition, y::UDisplacement) = UPosition(x.val + y.val)
Base.:+(x::UDisplacement, y::Displacement) = UDisplacement(x.val + y.val)
Base.:+(x::UDisplacement, y::UDisplacement) = UDisplacement(x.val + y.val)
checked_sub(x::UPosition, y::UPosition) = UDisplacement(checked_sub(x.val, y.val))
checked_sub(x::UPosition, y::UDisplacement) = UPosition(checked_sub(x.val, y.val))
checked_sub(x::UDisplacement, y::UDisplacement) = UDisplacement(checked_sub(x.val, y.val))
checked_add(x::UPosition, y::UDisplacement) = UPosition(checked_add(x.val, y.val))
checked_add(x::UDisplacement, y::UDisplacement) = UDisplacement(checked_add(x.val, y.val))
Base.:+(x::UPosition, y::Displacement) = UPosition(x.val + y.val)
Base.:(<=)(x::UPosition, y::UPosition) = x.val <= y.val
Base.:(<)(x::UPosition, y::UPosition) = x.val < y.val
Base.:(<)(x::UDisplacement, y::UDisplacement) = x.val < y.val

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

#Base.promote_rule(::Type{UDisplacement}, ::Type{Int}) = Int
#Base.convert(::Type{Int}, x::UDisplacement) = Int(x.val)

@testset "Ranges with nonstandard Integers" begin
for (start, stop) in [(2, 4), (3, 3), (3, -2)]
@test collect(Position(start) : Position(stop)) == Position.(start : stop)
end
r = Position(start) : Position(stop)
@test length(r) === Displacement(stop >= start ? stop - start + 1 : 0)
start >= 0 && stop >= 0 && @test UDisplacement(length(r).val) ===
checked_length(UPosition(start) : UPosition(stop)) ===
checked_length(UPosition(start) : Displacement(1) : UPosition(stop)) ===
checked_length(UPosition(start) : UDisplacement(1) : UPosition(stop))
@test collect(r) == Position.(start : stop)
end

@test length(UPosition(3):Displacement(7):UPosition(100)) === checked_length(UPosition(3):Displacement(7):UPosition(100)) === UDisplacement(14)
@test length(UPosition(100):Displacement(7):UPosition(3)) === checked_length(UPosition(100):Displacement(7):UPosition(3)) === UDisplacement(0)
@test length(UPosition(100):Displacement(-7):UPosition(3)) === checked_length(UPosition(100):Displacement(-7):UPosition(3)) === UDisplacement(14)
@test length(UPosition(3):Displacement(-7):UPosition(100)) === checked_length(UPosition(3):Displacement(-7):UPosition(100)) === UDisplacement(0)
@test_throws OverflowError checked_length(zero(UPosition):UPosition(typemax(UInt)))
@test_throws OverflowError checked_length(zero(UPosition):Displacement(1):UPosition(typemax(UInt)))
@test_throws OverflowError checked_length(UPosition(typemax(UInt)):Displacement(-1):zero(UPosition))

for start in [3, 0, -2]
@test collect(Base.OneTo(Position(start))) == Position.(Base.OneTo(start))
Expand All @@ -1555,7 +1642,7 @@ end
end # module NonStandardIntegerRangeTest

@testset "Issue #26619" begin
@test length(UInt(100) : -1 : 1) === UInt(100)
@test length(UInt(100) : -1 : 1) == checked_length(UInt(100) : -1 : 1) === UInt(100)
@test collect(UInt(5) : -1 : 3) == [UInt(5), UInt(4), UInt(3)]

let r = UInt(5) : -2 : 2
Expand Down

0 comments on commit 96a1e5f

Please sign in to comment.