Skip to content

Commit

Permalink
Deprecate [T]::rotate in favor of [T]::rotate_{left,right}.
Browse files Browse the repository at this point in the history
Background
==========

Slices currently have an unstable [`rotate`] method which rotates
elements in the slice to the _left_ N positions. [Here][tracking] is the
tracking issue for this unstable feature.

```rust
let mut a = ['a', 'b' ,'c', 'd', 'e', 'f'];
a.rotate(2);
assert_eq!(a, ['c', 'd', 'e', 'f', 'a', 'b']);
```

Proposal
========

Deprecate the [`rotate`] method and introduce `rotate_left` and
`rotate_right` methods.

```rust
let mut a = ['a', 'b' ,'c', 'd', 'e', 'f'];
a.rotate_left(2);
assert_eq!(a, ['c', 'd', 'e', 'f', 'a', 'b']);
```

```rust
let mut a = ['a', 'b' ,'c', 'd', 'e', 'f'];
a.rotate_right(2);
assert_eq!(a, ['e', 'f', 'a', 'b', 'c', 'd']);
```

Justification
=============

I used this method today for my first time and (probably because I’m a
naive westerner who reads LTR) was surprised when the docs mentioned that
elements get rotated in a left-ward direction. I was in a situation
where I needed to shift elements in a right-ward direction and had to
context switch from the main problem I was working on and think how much
to rotate left in order to accomplish the right-ward rotation I needed.

Ruby’s `Array.rotate` shifts left-ward, Python’s `deque.rotate` shifts
right-ward. Both of their implementations allow passing negative numbers
to shift in the opposite direction respectively.

Introducing `rotate_left` and `rotate_right` would:

- remove ambiguity about direction (alleviating need to read docs 😉)
- make it easier for people who need to rotate right

[`rotate`]: https://doc.rust-lang.org/std/primitive.slice.html#method.rotate
[tracking]: #41891
  • Loading branch information
frewsxcv committed Dec 25, 2017
1 parent ae65dcc commit 66ef6b9
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 50 deletions.
2 changes: 1 addition & 1 deletion src/liballoc/benches/slice.rs
Expand Up @@ -343,7 +343,7 @@ macro_rules! rotate {
fn $name(b: &mut Bencher) {
let size = mem::size_of_val(&$gen(1)[0]);
let mut v = $gen($len * 8 / size);
b.iter(|| black_box(&mut v).rotate(($mid*8+size-1)/size));
b.iter(|| black_box(&mut v).rotate_left(($mid*8+size-1)/size));
b.bytes = (v.len() * size) as u64;
}
}
Expand Down
99 changes: 64 additions & 35 deletions src/liballoc/slice.rs
Expand Up @@ -1360,24 +1360,61 @@ impl<T> [T] {
core_slice::SliceExt::sort_unstable_by_key(self, f);
}

/// Permutes the slice in-place such that `self[mid..]` moves to the
/// beginning of the slice while `self[..mid]` moves to the end of the
/// slice. Equivalently, rotates the slice `mid` places to the left
/// or `k = self.len() - mid` places to the right.
/// Rotates the slice in-place such that the first `mid` elements of the
/// slice move to the end while the last `self.len() - mid` elements move to
/// the front. After calling `rotate_left`, the element previously at index
/// `mid` will become the first element in the slice.
///
/// This is a "k-rotation", a permutation in which item `i` moves to
/// position `i + k`, modulo the length of the slice. See _Elements
/// of Programming_ [§10.4][eop].
/// # Panics
///
/// This function will panic if `mid` is greater than the length of the
/// slice. Note that `mid == self.len()` does _not_ panic and is a no-op
/// rotation.
///
/// # Complexity
///
/// Takes linear (in `self.len()`) time.
///
/// # Examples
///
/// Rotation by `mid` and rotation by `k` are inverse operations.
/// ```
/// #![feature(slice_rotate)]
///
/// [eop]: https://books.google.com/books?id=CO9ULZGINlsC&pg=PA178&q=k-rotation
/// let mut a = ['a', 'b', 'c', 'd', 'e', 'f'];
/// a.rotate_left(2);
/// assert_eq!(a, ['c', 'd', 'e', 'f', 'a', 'b']);
/// ```
///
/// Rotating a subslice:
///
/// ```
/// #![feature(slice_rotate)]
///
/// let mut a = ['a', 'b', 'c', 'd', 'e', 'f'];
/// a[1..5].rotate_left(1);
/// assert_eq!(a, ['a', 'c', 'd', 'e', 'b', 'f']);
/// ```
#[unstable(feature = "slice_rotate", issue = "41891")]
pub fn rotate_left(&mut self, mid: usize) {
core_slice::SliceExt::rotate_left(self, mid);
}

#[unstable(feature = "slice_rotate", issue = "41891")]
#[rustc_deprecated(since = "", reason = "renamed to `rotate_left`")]
pub fn rotate(&mut self, mid: usize) {
core_slice::SliceExt::rotate_left(self, mid);
}

/// Rotates the slice in-place such that the first `self.len() - k`
/// elements of the slice move to the end while the last `k` elements move
/// to the front. After calling `rotate_right`, the element previously at
/// index `self.len() - k` will become the first element in the slice.
///
/// # Panics
///
/// This function will panic if `mid` is greater than the length of the
/// slice. (Note that `mid == self.len()` does _not_ panic; it's a nop
/// rotation with `k == 0`, the inverse of a rotation with `mid == 0`.)
/// This function will panic if `k` is greater than the length of the
/// slice. Note that `k == self.len()` does _not_ panic and is a no-op
/// rotation.
///
/// # Complexity
///
Expand All @@ -1388,31 +1425,23 @@ impl<T> [T] {
/// ```
/// #![feature(slice_rotate)]
///
/// let mut a = [1, 2, 3, 4, 5, 6, 7];
/// let mid = 2;
/// a.rotate(mid);
/// assert_eq!(&a, &[3, 4, 5, 6, 7, 1, 2]);
/// let k = a.len() - mid;
/// a.rotate(k);
/// assert_eq!(&a, &[1, 2, 3, 4, 5, 6, 7]);
///
/// use std::ops::Range;
/// fn slide<T>(slice: &mut [T], range: Range<usize>, to: usize) {
/// if to < range.start {
/// slice[to..range.end].rotate(range.start-to);
/// } else if to > range.end {
/// slice[range.start..to].rotate(range.end-range.start);
/// }
/// }
/// let mut v: Vec<_> = (0..10).collect();
/// slide(&mut v, 1..4, 7);
/// assert_eq!(&v, &[0, 4, 5, 6, 1, 2, 3, 7, 8, 9]);
/// slide(&mut v, 6..8, 1);
/// assert_eq!(&v, &[0, 3, 7, 4, 5, 6, 1, 2, 8, 9]);
/// let mut a = ['a', 'b', 'c', 'd', 'e', 'f'];
/// a.rotate_right(2);
/// assert_eq!(a, ['e', 'f', 'a', 'b', 'c', 'd']);
/// ```
///
/// Rotate a subslice:
///
/// ```
/// #![feature(slice_rotate)]
///
/// let mut a = ['a', 'b', 'c', 'd', 'e', 'f'];
/// a[1..5].rotate_right(1);
/// assert_eq!(a, ['a', 'e', 'b', 'c', 'd', 'f']);
/// ```
#[unstable(feature = "slice_rotate", issue = "41891")]
pub fn rotate(&mut self, mid: usize) {
core_slice::SliceExt::rotate(self, mid);
pub fn rotate_right(&mut self, k: usize) {
core_slice::SliceExt::rotate_right(self, k);
}

/// Copies the elements from `src` into `self`.
Expand Down
51 changes: 43 additions & 8 deletions src/liballoc/tests/slice.rs
Expand Up @@ -494,37 +494,72 @@ fn test_sort_stability() {
}

#[test]
fn test_rotate() {
fn test_rotate_left() {
let expected: Vec<_> = (0..13).collect();
let mut v = Vec::new();

// no-ops
v.clone_from(&expected);
v.rotate(0);
v.rotate_left(0);
assert_eq!(v, expected);
v.rotate(expected.len());
v.rotate_left(expected.len());
assert_eq!(v, expected);
let mut zst_array = [(), (), ()];
zst_array.rotate(2);
zst_array.rotate_left(2);

// happy path
v = (5..13).chain(0..5).collect();
v.rotate(8);
v.rotate_left(8);
assert_eq!(v, expected);

let expected: Vec<_> = (0..1000).collect();

// small rotations in large slice, uses ptr::copy
v = (2..1000).chain(0..2).collect();
v.rotate(998);
v.rotate_left(998);
assert_eq!(v, expected);
v = (998..1000).chain(0..998).collect();
v.rotate(2);
v.rotate_left(2);
assert_eq!(v, expected);

// non-small prime rotation, has a few rounds of swapping
v = (389..1000).chain(0..389).collect();
v.rotate(1000-389);
v.rotate_left(1000-389);
assert_eq!(v, expected);
}

#[test]
fn test_rotate_right() {
let expected: Vec<_> = (0..13).collect();
let mut v = Vec::new();

// no-ops
v.clone_from(&expected);
v.rotate_right(0);
assert_eq!(v, expected);
v.rotate_right(expected.len());
assert_eq!(v, expected);
let mut zst_array = [(), (), ()];
zst_array.rotate_right(2);

// happy path
v = (5..13).chain(0..5).collect();
v.rotate_right(5);
assert_eq!(v, expected);

let expected: Vec<_> = (0..1000).collect();

// small rotations in large slice, uses ptr::copy
v = (2..1000).chain(0..2).collect();
v.rotate_right(2);
assert_eq!(v, expected);
v = (998..1000).chain(0..998).collect();
v.rotate_right(998);
assert_eq!(v, expected);

// non-small prime rotation, has a few rounds of swapping
v = (389..1000).chain(0..389).collect();
v.rotate_right(389);
assert_eq!(v, expected);
}

Expand Down
17 changes: 15 additions & 2 deletions src/libcore/slice/mod.rs
Expand Up @@ -201,7 +201,10 @@ pub trait SliceExt {
fn ends_with(&self, needle: &[Self::Item]) -> bool where Self::Item: PartialEq;

#[unstable(feature = "slice_rotate", issue = "41891")]
fn rotate(&mut self, mid: usize);
fn rotate_left(&mut self, mid: usize);

#[unstable(feature = "slice_rotate", issue = "41891")]
fn rotate_right(&mut self, k: usize);

#[stable(feature = "clone_from_slice", since = "1.7.0")]
fn clone_from_slice(&mut self, src: &[Self::Item]) where Self::Item: Clone;
Expand Down Expand Up @@ -640,7 +643,7 @@ impl<T> SliceExt for [T] {
self.binary_search_by(|p| p.cmp(x))
}

fn rotate(&mut self, mid: usize) {
fn rotate_left(&mut self, mid: usize) {
assert!(mid <= self.len());
let k = self.len() - mid;

Expand All @@ -650,6 +653,16 @@ impl<T> SliceExt for [T] {
}
}

fn rotate_right(&mut self, k: usize) {
assert!(k <= self.len());
let mid = self.len() - k;

unsafe {
let p = self.as_mut_ptr();
rotate::ptr_rotate(mid, p.offset(mid as isize), k);
}
}

#[inline]
fn clone_from_slice(&mut self, src: &[T]) where T: Clone {
assert!(self.len() == src.len(),
Expand Down
21 changes: 18 additions & 3 deletions src/libcore/tests/slice.rs
Expand Up @@ -290,17 +290,32 @@ fn test_iter_folds() {
}

#[test]
fn test_rotate() {
fn test_rotate_left() {
const N: usize = 600;
let a: &mut [_] = &mut [0; N];
for i in 0..N {
a[i] = i;
}

a.rotate(42);
a.rotate_left(42);
let k = N - 42;

for i in 0..N {
assert_eq!(a[(i+k)%N], i);
assert_eq!(a[(i + k) % N], i);
}
}

#[test]
fn test_rotate_right() {
const N: usize = 600;
let a: &mut [_] = &mut [0; N];
for i in 0..N {
a[i] = i;
}

a.rotate_right(42);

for i in 0..N {
assert_eq!(a[(i + 42) % N], i);
}
}
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest.rs
Expand Up @@ -2856,7 +2856,7 @@ fn read2_abbreviated(mut child: Child) -> io::Result<Output> {
*skipped += data.len();
if data.len() <= TAIL_LEN {
tail[..data.len()].copy_from_slice(data);
tail.rotate(data.len());
tail.rotate_left(data.len());
} else {
tail.copy_from_slice(&data[(data.len() - TAIL_LEN)..]);
}
Expand Down

0 comments on commit 66ef6b9

Please sign in to comment.