From 68e120fbab58a3a946ba4cf1920e5d98d68889f7 Mon Sep 17 00:00:00 2001 From: Matt Bauman Date: Thu, 6 Jan 2022 15:19:31 -0500 Subject: [PATCH] Fix reverse of ranges of unsigned integers (#29842) Fixes #29576 Co-authored-by: Jameson Nash Co-authored-by: Lilith Orion Hafner <60898866+LilithHafner@users.noreply.github.com> --- base/broadcast.jl | 14 +++++++------- base/int.jl | 4 ++++ base/range.jl | 6 +++--- test/ranges.jl | 14 +++++++++++--- test/sorting.jl | 3 ++- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/base/broadcast.jl b/base/broadcast.jl index 85dc56c578763..b123510cdb476 100644 --- a/base/broadcast.jl +++ b/base/broadcast.jl @@ -9,7 +9,7 @@ module Broadcast using .Base.Cartesian using .Base: Indices, OneTo, tail, to_shape, isoperator, promote_typejoin, promote_typejoin_union, @pure, - _msk_end, unsafe_bitgetindex, bitcache_chunks, bitcache_size, dumpbitcache, unalias + _msk_end, unsafe_bitgetindex, bitcache_chunks, bitcache_size, dumpbitcache, unalias, negate import .Base: copy, copyto!, axes export broadcast, broadcast!, BroadcastStyle, broadcast_axes, broadcastable, dotview, @__dot__, BroadcastFunction @@ -1079,9 +1079,9 @@ end # DefaultArrayStyle and \ are not available at the time of range.jl broadcasted(::DefaultArrayStyle{1}, ::typeof(+), r::AbstractRange) = r -broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::AbstractRange) = range(-first(r), step=-step(r), length=length(r)) -broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::OrdinalRange) = range(-first(r), -last(r), step=-step(r)) -broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::StepRangeLen) = StepRangeLen(-r.ref, -r.step, length(r), r.offset) +broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::AbstractRange) = range(-first(r), step=negate(step(r)), length=length(r)) +broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::OrdinalRange) = range(-first(r), -last(r), step=negate(step(r))) +broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::StepRangeLen) = StepRangeLen(-r.ref, negate(r.step), length(r), r.offset) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::LinRange) = LinRange(-r.start, -r.stop, length(r)) # For #18336 we need to prevent promotion of the step type: @@ -1102,15 +1102,15 @@ broadcasted(::DefaultArrayStyle{1}, ::typeof(+), x::Number, r::LinRange) = LinRa broadcasted(::DefaultArrayStyle{1}, ::typeof(+), r1::AbstractRange, r2::AbstractRange) = r1 + r2 broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::AbstractRange, x::Number) = range(first(r) - x, step=step(r), length=length(r)) -broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Number, r::AbstractRange) = range(x - first(r), step=-step(r), length=length(r)) +broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Number, r::AbstractRange) = range(x - first(r), step=negate(step(r)), length=length(r)) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::OrdinalRange, x::Integer) = range(first(r) - x, last(r) - x, step=step(r)) -broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Integer, r::OrdinalRange) = range(x - first(r), x - last(r), step=-step(r)) +broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Integer, r::OrdinalRange) = range(x - first(r), x - last(r), step=negate(step(r))) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::AbstractUnitRange, x::Integer) = range(first(r) - x, last(r) - x) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::AbstractUnitRange, x::Real) = range(first(r) - x, length=length(r)) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::StepRangeLen{T}, x::Number) where T = StepRangeLen{typeof(T(r.ref)-x)}(r.ref - x, r.step, length(r), r.offset) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Number, r::StepRangeLen{T}) where T = - StepRangeLen{typeof(x-T(r.ref))}(x - r.ref, -r.step, length(r), r.offset) + StepRangeLen{typeof(x-T(r.ref))}(x - r.ref, negate(r.step), length(r), r.offset) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r::LinRange, x::Number) = LinRange(r.start - x, r.stop - x, length(r)) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), x::Number, r::LinRange) = LinRange(x - r.start, x - r.stop, length(r)) broadcasted(::DefaultArrayStyle{1}, ::typeof(-), r1::AbstractRange, r2::AbstractRange) = r1 - r2 diff --git a/base/int.jl b/base/int.jl index 49ec0c54721f1..4f0b2877c0591 100644 --- a/base/int.jl +++ b/base/int.jl @@ -87,6 +87,10 @@ signed(::Type{T}) where {T<:Signed} = T (+)(x::T, y::T) where {T<:BitInteger} = add_int(x, y) (*)(x::T, y::T) where {T<:BitInteger} = mul_int(x, y) +negate(x) = -x +negate(x::Unsigned) = -convert(Signed, x) +#widenegate(x) = -convert(widen(signed(typeof(x))), x) + inv(x::Integer) = float(one(x)) / float(x) (/)(x::T, y::T) where {T<:Integer} = float(x) / float(y) # skip promotion for system integer types diff --git a/base/range.jl b/base/range.jl index f6a4356b16508..5c777c57faf17 100644 --- a/base/range.jl +++ b/base/range.jl @@ -1246,7 +1246,7 @@ issubset(r::AbstractUnitRange{<:Integer}, s::AbstractUnitRange{<:Integer}) = ## linear operations on ranges ## --(r::OrdinalRange) = range(-first(r), step=-step(r), length=length(r)) +-(r::OrdinalRange) = range(-first(r), step=negate(step(r)), length=length(r)) -(r::StepRangeLen{T,R,S,L}) where {T,R,S,L} = StepRangeLen{T,R,S,L}(-r.ref, -r.step, r.len, r.offset) function -(r::LinRange) @@ -1352,13 +1352,13 @@ end Array{T,1}(r::AbstractRange{T}) where {T} = vcat(r) collect(r::AbstractRange) = vcat(r) -_reverse(r::OrdinalRange, ::Colon) = (:)(last(r), -step(r), first(r)) +_reverse(r::OrdinalRange, ::Colon) = (:)(last(r), negate(step(r)), first(r)) function _reverse(r::StepRangeLen, ::Colon) # If `r` is empty, `length(r) - r.offset + 1 will be nonpositive hence # invalid. As `reverse(r)` is also empty, any offset would work so we keep # `r.offset` offset = isempty(r) ? r.offset : length(r)-r.offset+1 - return typeof(r)(r.ref, -r.step, length(r), offset) + return typeof(r)(r.ref, negate(r.step), length(r), offset) end _reverse(r::LinRange{T}, ::Colon) where {T} = typeof(r)(r.stop, r.start, length(r)) diff --git a/test/ranges.jl b/test/ranges.jl index b0306ac68b33d..06242bda2af09 100644 --- a/test/ranges.jl +++ b/test/ranges.jl @@ -382,6 +382,14 @@ end @test reverse(reverse(typemin(Int):typemax(Int))) == typemin(Int):typemax(Int) @test reverse(reverse(typemin(Int):2:typemax(Int))) == typemin(Int):2:typemax(Int) end + @testset "reverse `[Step|Unit]Range{$T}`" for T in (Int8, UInt8, Int, UInt, Int128, UInt128) + @test reverse(T(1):T(10)) == T(10):-1:T(1) + @test reverse(typemin(T):typemax(T)) == typemax(T):-1:typemin(T) + @test reverse(typemin(T):2:typemax(T)) == typemax(T)-T(1):-2:typemin(T) + @test reverse(reverse(T(1):T(10))) == T(1):T(10) + @test reverse(reverse(typemin(T):typemax(T))) == typemin(T):typemax(T) + @test reverse(reverse(typemin(T):2:typemax(T))) == typemin(T):2:typemax(T) + end @testset "intersect" begin @test intersect(1:5, 2:3) === 2:3 @test intersect(-3:5, 2:8) === 2:5 @@ -717,9 +725,9 @@ end @test broadcast(-, T(1):2:6, 1) === T(0):2:4 @test broadcast(-, T(1):2:6, 0.3) === range(T(1)-0.3, step=2, length=T(3)) == T(1)-0.3:2:5-0.3 is_unsigned = T <: Unsigned - is_unsigned && @test length(broadcast(-, T(1):3, 2)) === length(T(1)-2:T(3)-2) - @test broadcast(-, T(1):3) == -T(1):-T(1):-T(3) - @test broadcast(-, 2, T(1):3) == T(1):-T(1):-T(1) + @test length(broadcast(-, T(1):3, 2)) === length(T(1)-2:T(3)-2) === (is_unsigned ? T(0) : T(3)) + @test broadcast(-, T(1):3) == -T(1):-1:-T(3) + @test broadcast(-, 2, T(1):3) == T(1):-1:-T(1) end @testset "operations between ranges and arrays" for T in (Int, UInt, Int128) @test all(([T(1):5;] + (T(5):-1:1)) .=== T(6)) diff --git a/test/sorting.jl b/test/sorting.jl index e90138549afd8..6deacadc2a169 100644 --- a/test/sorting.jl +++ b/test/sorting.jl @@ -56,6 +56,8 @@ end @test issorted([1,2,3]) @test reverse([2,3,1]) == [1,3,2] @test sum(randperm(6)) == 21 + @test length(reverse(0x1:0x2)) == 2 + @test issorted(sort(rand(UInt64(1):UInt64(2), 7); rev=true); rev=true) # issue #43034 end @testset "partialsort" begin @@ -220,7 +222,6 @@ end end end - @test_broken length(reverse(0x1:0x2)) == 2 @testset "issue #34408" begin r = 1f8-10:1f8 # collect(r) = Float32[9.999999e7, 9.999999e7, 9.999999e7, 9.999999e7, 1.0e8, 1.0e8, 1.0e8, 1.0e8, 1.0e8]