Skip to content

Commit

Permalink
Fix reverse of ranges of unsigned integers
Browse files Browse the repository at this point in the history
Fixes #29576

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
  • Loading branch information
mbauman and vtjnash committed Nov 11, 2021
1 parent c25b704 commit 68afe07
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 13 deletions.
14 changes: 7 additions & 7 deletions base/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand All @@ -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
Expand Down
4 changes: 4 additions & 0 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,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)
Expand Down Expand Up @@ -1340,13 +1340,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))

Expand Down
14 changes: 11 additions & 3 deletions test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,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
Expand Down Expand Up @@ -699,9 +707,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))
Expand Down

0 comments on commit 68afe07

Please sign in to comment.