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

Deprecate rol and ror in favor of circshift methods #23404

Merged
merged 2 commits into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,9 @@ Deprecated or removed
* `filter` and `filter!` on dictionaries now pass a single `key=>value` pair to the
argument function, instead of two arguments ([#17886]).

* `rol`, `rol!`, `ror`, and `ror!` have been deprecated in favor of specialized methods for
`circshift`/`circshift!` ([#23404]).

* `Base.SparseArrays.SpDiagIterator` has been removed ([#23261]).

* The tuple-of-types form of `cfunction`, `cfunction(f, returntype, (types...))`, has been deprecated
Expand Down Expand Up @@ -1247,3 +1250,4 @@ Command-line option changes
[#23207]: https://github.com/JuliaLang/julia/issues/23207
[#23233]: https://github.com/JuliaLang/julia/issues/23233
[#23342]: https://github.com/JuliaLang/julia/issues/23342
[#23404]: https://github.com/JuliaLang/julia/issues/23404
29 changes: 27 additions & 2 deletions base/abstractarraymath.jl
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,9 @@ circshift(a::AbstractArray, shiftamt::DimsInteger) = circshift!(similar(a), a, s
"""
circshift(A, shifts)

Circularly shift the data in an array. The second argument is a vector giving the amount to
shift in each dimension.
Circularly shift, i.e. rotate, the data in an array. The second argument is a tuple or
vector giving the amount to shift in each dimension, or an integer to shift only in the
first dimension.

# Examples
```jldoctest
Expand All @@ -203,6 +204,30 @@ julia> circshift(b, (-1,0))
3 7 11 15
4 8 12 16
1 5 9 13

julia> a = BitArray([true, true, false, false, true])
5-element BitArray{1}:
true
true
false
false
true

julia> circshift(a, 1)
5-element BitArray{1}:
true
true
true
false
false

julia> circshift(a, -1)
5-element BitArray{1}:
true
false
false
true
true
```

See also [`circshift!`](@ref).
Expand Down
141 changes: 10 additions & 131 deletions base/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1478,145 +1478,24 @@ details and examples.
"""
(>>>)(B::BitVector, i::Int) = (i >=0 ? B >> unsigned(i) : B << unsigned(-i))

"""
rol!(dest::BitVector, src::BitVector, i::Integer) -> BitVector

Performs a left rotation operation on `src` and puts the result into `dest`.
`i` controls how far to rotate the bits.
"""
function rol!(dest::BitVector, src::BitVector, i::Integer)
length(dest) == length(src) || throw(ArgumentError("destination and source should be of same size"))
n = length(dest)
i %= n
i == 0 && return (src === dest ? src : copy!(dest, src))
i < 0 && return ror!(dest, src, -i)
Bc = (src === dest ? copy(src.chunks) : src.chunks)
copy_chunks!(dest.chunks, 1, Bc, i+1, n-i)
copy_chunks!(dest.chunks, n-i+1, Bc, 1, i)
return dest
end

"""
rol!(B::BitVector, i::Integer) -> BitVector

Performs a left rotation operation in-place on `B`.
`i` controls how far to rotate the bits.
"""
rol!(B::BitVector, i::Integer) = rol!(B, B, i)

"""
rol(B::BitVector, i::Integer) -> BitVector

Performs a left rotation operation, returning a new `BitVector`.
`i` controls how far to rotate the bits.
See also [`rol!`](@ref).

# Examples
```jldoctest
julia> A = BitArray([true, true, false, false, true])
5-element BitArray{1}:
true
true
false
false
true

julia> rol(A,1)
5-element BitArray{1}:
true
false
false
true
true

julia> rol(A,2)
5-element BitArray{1}:
false
false
true
true
true

julia> rol(A,5)
5-element BitArray{1}:
true
true
false
false
true
```
"""
rol(B::BitVector, i::Integer) = rol!(similar(B), B, i)

"""
ror!(dest::BitVector, src::BitVector, i::Integer) -> BitVector

Performs a right rotation operation on `src` and puts the result into `dest`.
`i` controls how far to rotate the bits.
"""
function ror!(dest::BitVector, src::BitVector, i::Integer)
function circshift!(dest::BitVector, src::BitVector, i::Integer)
length(dest) == length(src) || throw(ArgumentError("destination and source should be of same size"))
n = length(dest)
i %= n
i == 0 && return (src === dest ? src : copy!(dest, src))
i < 0 && return rol!(dest, src, -i)
Bc = (src === dest ? copy(src.chunks) : src.chunks)
copy_chunks!(dest.chunks, i+1, Bc, 1, n-i)
copy_chunks!(dest.chunks, 1, Bc, n-i+1, i)
if i > 0 # right
copy_chunks!(dest.chunks, i+1, Bc, 1, n-i)
copy_chunks!(dest.chunks, 1, Bc, n-i+1, i)
else # left
i = -i
copy_chunks!(dest.chunks, 1, Bc, i+1, n-i)
copy_chunks!(dest.chunks, n-i+1, Bc, 1, i)
end
return dest
end

"""
ror!(B::BitVector, i::Integer) -> BitVector

Performs a right rotation operation in-place on `B`.
`i` controls how far to rotate the bits.
"""
ror!(B::BitVector, i::Integer) = ror!(B, B, i)

"""
ror(B::BitVector, i::Integer) -> BitVector

Performs a right rotation operation on `B`, returning a new `BitVector`.
`i` controls how far to rotate the bits.
See also [`ror!`](@ref).

# Examples
```jldoctest
julia> A = BitArray([true, true, false, false, true])
5-element BitArray{1}:
true
true
false
false
true

julia> ror(A,1)
5-element BitArray{1}:
true
true
true
false
false

julia> ror(A,2)
5-element BitArray{1}:
false
true
true
true
false

julia> ror(A,5)
5-element BitArray{1}:
true
true
false
false
true
```
"""
ror(B::BitVector, i::Integer) = ror!(similar(B), B, i)
circshift!(B::BitVector, i::Integer) = circshift!(B, B, i)
Copy link
Member

@stevengj stevengj Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a single-array circshift! for non-bitvectors? I guess not, but it's a bit weird to have this only for one specialized array type (maybe re-open #16032).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've reopened #16032. Is the disconnect in methods blocking for this PR or can that be added later?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do that separately.


## count & find ##

Expand Down
7 changes: 7 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1668,6 +1668,13 @@ export hex2num
@deprecate convert(::Type{String}, v::Vector{UInt8}) String(v)
@deprecate convert(::Type{S}, g::UTF8proc.GraphemeIterator) where {S<:AbstractString} convert(S, g.s)

# Issue #19923
@deprecate ror circshift
@deprecate ror! circshift!
@deprecate rol(B, i) circshift(B, -i)
@deprecate rol!(dest, src, i) circshift!(dest, src, -i)
@deprecate rol!(B, i) circshift!(B, -i)

# issue #5148, PR #23259
# warning for `const` on locals should be changed to an error in julia-syntax.scm

Expand Down
4 changes: 0 additions & 4 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,6 @@ export
# bitarrays
falses,
flipbits!,
rol,
rol!,
ror,
ror!,
trues,

# dequeues
Expand Down
2 changes: 1 addition & 1 deletion base/multidimensional.jl
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ circshift!(dest::AbstractArray, src, ::Tuple{}) = copy!(dest, src)
"""
circshift!(dest, src, shifts)

Circularly shift the data in `src`, storing the result in
Circularly shift, i.e. rotate, the data in `src`, storing the result in
`dest`. `shifts` specifies the amount to shift in each dimension.

The `dest` array must be distinct from the `src` array (they cannot
Expand Down
4 changes: 0 additions & 4 deletions doc/src/stdlib/arrays.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,6 @@ and can be converted to/from the latter via `Array(bitarray)` and `BitArray(arra

```@docs
Base.flipbits!
Base.rol!
Base.rol
Base.ror!
Base.ror
```

## [Sparse Vectors and Matrices](@id stdlib-sparse-arrays)
Expand Down
15 changes: 7 additions & 8 deletions test/bitarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1040,21 +1040,20 @@ timesofar("binary comparison")
@test isequal(b1 >>> m, [ falses(m); b1[1:end-m] ])
@test isequal(b1 << -m, b1 >> m)
@test isequal(b1 >>> -m, b1 << m)
@test isequal(rol(b1, m), [ b1[m+1:end]; b1[1:m] ])
@test isequal(ror(b1, m), [ b1[end-m+1:end]; b1[1:end-m] ])
@test isequal(ror(b1, m), rol(b1, -m))
@test isequal(rol(b1, m), ror(b1, -m))
@test isequal(circshift(b1, -m), [ b1[m+1:end]; b1[1:m] ])
@test isequal(circshift(b1, m), [ b1[end-m+1:end]; b1[1:end-m] ])
@test isequal(circshift(b1, m), circshift(b1, m - length(b1)))
end

b = bitrand(v1)
i = bitrand(v1)
for m = [rand(1:v1), 63, 64, 65, 191, 192, 193, v1-1]
j = rand(1:m)
b1 = ror!(i, b, j)
i1 = ror!(b, j)
b1 = circshift!(i, b, j)
i1 = circshift!(b, j)
@test b1 == i1
b2 = rol!(i1, b1, j)
i2 = rol!(b1, j)
b2 = circshift!(i1, b1, -j)
i2 = circshift!(b1, -j)
@test b2 == i2
end
end
Expand Down