Deprecate rol and ror in favor of circshift methods (#23404)

Author: ararslan

NEWS.md

@@ -372,6 +372,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
@@ -1269,3 +1272,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

base/abstractarraymath.jl

@@ -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
@@ -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).

base/bitarray.jl

@@ -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)
 
 ## count & find ##
 

base/deprecated.jl

@@ -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
 

base/exports.jl

@@ -638,10 +638,6 @@ export
 # bitarrays
     falses,
     flipbits!,
-    rol,
-    rol!,
-    ror,
-    ror!,
     trues,
 
 # dequeues

base/multidimensional.jl

@@ -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

doc/src/stdlib/arrays.md

@@ -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)

test/bitarray.jl

@@ -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