# Remove panic from rotate\_left and rotate\_right?

**URL:** <https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480>\
**Category:** language design\
**Created:** [March 16, 2024, 2:05am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480 "2024-03-16T02:05:44Z")\
**Posts on this page:** 20\
**Page:** 1

<div class="post-metadata">

**Author:** ![spencer3035](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/spencer3035/32/11936_2.png) [@spencer3035](https://internals.rust-lang.org/u/spencer3035)\
**Post date:** [March 16, 2024, 2:05am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/1 "2024-03-16T02:05:44Z")

</div>

It seems unnessasry to me to have a panic for these functions for slices. Why can they not just wrap around as expected? I did a quick test of the implementations and checked everything worked.

I did the change here: [GitHub - spencer3035/rust: Empowering everyone to build reliable and efficient software.](https://github.com/spencer3035/rust)

---

<div class="post-metadata">

**Author:** ![dlight](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dlight/32/8462_2.png) [@dlight](https://internals.rust-lang.org/u/dlight)\
**Post date:** [March 16, 2024, 4:53am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/2 "2024-03-16T04:53:47Z")

</div>

In this case it would be more appropriate to add wrapped methods, like `wrapping_rotate_left` and `wrapping_rotate_right`, and maybe a checked variant as well (`checked_rotate_left` and `checked_rotate_right`), in analogy to `wrapping_add` and `checked_add` for integers in the stdlib.

---

<div class="post-metadata">

**Author:** ![steffahn](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/steffahn/32/13288_2.png) [@steffahn](https://internals.rust-lang.org/u/steffahn)\
**Post date:** [March 16, 2024, 8:52am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/3 "2024-03-16T08:52:40Z")

</div>

Removing a documented panic case from standard library APIs because it "seems unnecessary" is a bit like removing the panic from the `assert!` macro because it "seems unnecessary". Surely, one can just remove that pescy panic `assert!(false)` produces, as the rest of its implementation doesn't need it, right? No. Users could have been relying on that (documented) panic to make their program safe against invalid states or adverserial inputs!

---

<div class="post-metadata">

**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\
**Post date:** [March 16, 2024, 6:17pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/4 "2024-03-16T18:17:26Z")

</div>

If you want modulo, you should call it yourself. `%` by a runtime value is surprisingly expensive -- _far_ more than the easily predicted assert, on modern platforms -- so there's no reason to impose that cost on everyone doing the normal thing of just not making such a big index in the first place.

(Why do you even want to have it wrap? How'd you end up in that scenario?)

---

<div class="post-metadata">

**Author:** ![jeanas](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/jeanas/32/10430_2.png) [@jeanas](https://internals.rust-lang.org/u/jeanas)\
**Post date:** [March 16, 2024, 6:38pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/5 "2024-03-16T18:38:12Z")

</div>

What's the reason you think these functions are "expected" to wrap around? What's the difference with, say, slice indexing, which doesn't wrap around?

---

<div class="post-metadata">

**Author:** ![jrose](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/jrose/32/9591_2.png) [@jrose](https://internals.rust-lang.org/u/jrose)\
**Post date:** [March 16, 2024, 6:47pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/6 "2024-03-16T18:47:57Z")

</div>

I think having them wrap around is mathematically reasonable; rotation is a modular operation, where if you rotate a list of N items N times individually, you get back the original list.

I also think the cost of `%` is still relevant, and additionally the presence of a rotation amount larger than the slice size could easily indicate a bug in a program that’s better complained about than glossed over. Adding `%` at the call site is possible; removing it from within the callee would only happen via compiler optimization if the rotation amount is a constant.

---

<div class="post-metadata">

**Author:** ![quaternic](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/quaternic/32/10440_2.png) [@quaternic](https://internals.rust-lang.org/u/quaternic)\
**Post date:** [March 16, 2024, 7:32pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/7 "2024-03-16T19:32:24Z")

</div>

I find the expectation reasonable; integer rotations (of bits) do have that behaviour in Rust.

I don't quite buy the performance argument either: The panicking behaviour requires branching on `n > slice.len()` anyway, so making that branch instead compute the remainder and resume wouldn't hurt current code any more than removing an `assert!(n <= slice.len())` would.

However, preserving documented stable behaviour seems more important. Are there any other examples where panics on some inputs have been removed later?

edit: Oh, slice rotates actually allow `n == v.len()` (behaving the same as `n == 0`), so a rotation not panicking does not even mean the argument is a valid index to the slice.

---

<div class="post-metadata">

**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\
**Post date:** [March 16, 2024, 7:40pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/8 "2024-03-16T19:40:08Z")

</div>

> [@jrose](#):
>
> additionally the presence of a rotation amount larger than the slice size could easily indicate a bug in a program that’s better complained about than glossed over.

_This_, especially because it takes `usize`.

If you compute the rotation amount in such a way that you get `-1 as usize`, that will have a jump in the behaviour for any slice length that's not a power of two.

Run this and observe the jump in behaviour: [https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=513898cf45d7603a980b5afa741aab35](https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=513898cf45d7603a980b5afa741aab35)

(integer rotations don't have that problem because the integers are always power-of-two bits today)

---

<div class="post-metadata">

**Author:** ![spencer3035](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/spencer3035/32/11936_2.png) [@spencer3035](https://internals.rust-lang.org/u/spencer3035)\
**Post date:** [March 16, 2024, 10:28pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/9 "2024-03-16T22:28:24Z")

</div>

Besides that behavior in other languages, the term "rotate" would imply that it can be done indefinately. Especially that the rotation of `vec.len()` is valid seems to imply that as well.

---

<div class="post-metadata">

**Author:** ![spencer3035](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/spencer3035/32/11936_2.png) [@spencer3035](https://internals.rust-lang.org/u/spencer3035)\
**Post date:** [March 16, 2024, 10:58pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/10 "2024-03-16T22:58:16Z")

</div>

Thank you for all the replies.

It seems inconclusive if the modulo operation would result in more performance loss than the assert. I may do some testing later on this.

I partially understand the argument that it should remain for stability reasons (although I would disagree with someone who was relying on a panic rather than proper error handling). But I also see value in making functions take the largest reasonable range of inputs to make the user experience more streamlined.

Perhaps an alternative compromise would be to make a `rotate()` function similar to the following:

```rust
pub fn rotate(&mut self, rot: isize) {
    if self.is_empty() {
        return;
    }
    if rot >= 0 {
        let rot = (rot % self.len()) as usize;
        self.rotate_right(rot);
    } else {
        let rot = (rot.abs() % self.len()) as usize;
        self.rotate_left(rot);
    }
}

```

That way we get a function that does not panic and also has more generic behavior. I think this is more elegant than making a wrapping version of both existing rotate functions.

---

<div class="post-metadata">

**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\
**Post date:** [March 17, 2024, 12:07am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/11 "2024-03-17T00:07:04Z")

</div>

> [@spencer3035](#):
>
> Especially that the rotation of `vec.len()` is valid seems to imply that as well.

No, that's because it takes fenceposts, not indexes.

It's like how `my_slice.split_at(my_slice.len())` is legal. It's not modulo, just a fencepost.

---

<div class="post-metadata">

**Author:** ![dlight](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/dlight/32/8462_2.png) [@dlight](https://internals.rust-lang.org/u/dlight)\
**Post date:** [March 17, 2024, 3:20am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/12 "2024-03-17T03:20:13Z")

</div>

Maybe the wrapping version should be mod len + 1 then?

Rotating is an operation that should be possible to make periodic. If the number of elements is len, the number of fenceposts is len + 1

---

<div class="post-metadata">

**Author:** ![FZs](https://avatars.discourse-cdn.com/v4/letter/f/85f322/32.png) [@FZs](https://internals.rust-lang.org/u/FZs)\
**Post date:** [March 17, 2024, 12:34pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/13 "2024-03-17T12:34:15Z")

</div>

> [@spencer3035](#):
>
> ```rust
> pub fn rotate(&mut self, rot: isize) {
> if self.is_empty() {
> return;
> }
> if rot >= 0 {
> let rot = (rot % self.len()) as usize;
> self.rotate_right(rot);
> } else {
> let rot = (rot.abs() % self.len()) as usize;
> self.rotate_left(rot);
> }
> }
> 
> ```

I think such a method would make sense, although your implementation is overcomplicated. Exactly because of the modular nature of rotations, one can express it with either `rotate_left` or `rotate_right` only, using Euclidean division:

```rust
pub fn rotate(&mut self, rot: isize) {
    if self.is_empty() {
        return;
    }
    self.rotate_right(rot.rem_euclid(self.len() as isize) as usize)
}

```

[Playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b22713d1a9d74cbd9388330dfc01081f)

---

<div class="post-metadata">

**Author:** ![RalfJung](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ralfjung/32/2415_2.png) [@RalfJung](https://internals.rust-lang.org/u/RalfJung)\
**Post date:** [March 17, 2024, 6:56pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/14 "2024-03-17T18:56:16Z")

</div>

> [@scottmcm](#):
>
> No, that's because it takes fenceposts, not indexes.
> 
> It's like how `my_slice.split_at(my_slice.len())` is legal. It's not modulo, just a fencepost.

But why does it take a fencepost? "split" takes an index that points _between_ two elements, so there are clearly `len + 1` valid distinct inputs.

However, for `rotate`, there are only `len` distinct rotations to consider, so I would expect only `len` valid inputs (if wrap-around is not desired). The fact that `slice.rotate` accepts `len + 1` different inputs does seem very inconsistent to me. Conceptually rotating by `len` is the shortest rotate that "wraps around", so I would expect rotating by `len` and by `len + 1` to behave consistently -- either both go modulo `len` or neither does.

---

<div class="post-metadata">

**Author:** ![scottmcm](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/scottmcm/32/2355_2.png) [@scottmcm](https://internals.rust-lang.org/u/scottmcm)\
**Post date:** [March 17, 2024, 8:02pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/15 "2024-03-17T20:02:19Z")

</div>

> [@RalfJung](#):
>
> But why does it take a fencepost?

Well, mostly because I [originally](https://github.com/rust-lang/rust/pull/41670) wrote it as a single `rotate` (without `_left` and `_right` variants), following [http://elementsofprogramming.com/eop\_coloredlinks.pdf#section.10.4](http://elementsofprogramming.com/eop_coloredlinks.pdf#section.10.4), which takes a fencepost 🙃

I suppose the most direct answer for why that'd be fenceposts (not just indexes) is this Lemma:

> Lemma 10.21 The inverse of a k-rotation of n elements is an (n − k)-rotation.

which doesn't cover a zero-rotation unless an n-rotation of n elements is also legal.

And the simplest `rotate` implementation is

```rust
fn rotate<T>(x: &mut [T], k: usize) {
    x[..k].reverse();
    x[k..].reverse();
    x.reverse();
}

```

which naturally takes fenceposts, not indexes.

---

<div class="post-metadata">

**Author:** ![RalfJung](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ralfjung/32/2415_2.png) [@RalfJung](https://internals.rust-lang.org/u/RalfJung)\
**Post date:** [March 18, 2024, 6:49am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/16 "2024-03-18T06:49:44Z")

</div>

@scottmcm that seems like an implementation detail to me, given that the exposed interface is `rotate_left`/`rotate_right`.

> [@scottmcm](#):
>
> I suppose the most direct answer for why that'd be fenceposts (not just indexes) is this Lemma:
> 
> > Lemma 10.21 The inverse of a k-rotation of n elements is an (n − k)-rotation.
> 
> which doesn't cover a zero-rotation unless an n-rotation of n elements is also legal.

It also covers a zero rotation if `k` is an unbounded integer. 🙂 In fact the lemma becomes trivial then: the inverse of a `k` rotation is a `-k` rotation, and rotations that are equal modulo `n` are identical, so `-k` and `n-k` are the same.

---

<div class="post-metadata">

**Author:** ![SkiFire13](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/skifire13/32/7579_2.png) [@SkiFire13](https://internals.rust-lang.org/u/SkiFire13)\
**Post date:** [March 18, 2024, 11:53am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/17 "2024-03-18T11:53:18Z")

</div>

> [@RalfJung](#):
>
> But why does it take a fencepost? "split" takes an index that points _between_ two elements, so there are clearly `len + 1` valid distinct inputs.

Isn't "an index that points between two elements" a different, less precise, way to define a fencepost (since index `0` and `len` are not actually between two elements)?

---

<div class="post-metadata">

**Author:** ![RalfJung](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/ralfjung/32/2415_2.png) [@RalfJung](https://internals.rust-lang.org/u/RalfJung)\
**Post date:** [March 18, 2024, 1:00pm UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/18 "2024-03-18T13:00:32Z")

</div>

Yes, that is what a fencepost is. That's why fenceposts make sense for "split".

But that reasoning does not apply to rotations.

---

<div class="post-metadata">

**Author:** ![spencer3035](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/spencer3035/32/11936_2.png) [@spencer3035](https://internals.rust-lang.org/u/spencer3035)\
**Post date:** [March 19, 2024, 5:47am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/19 "2024-03-19T05:47:03Z")

</div>

This is a really elegant solution. I think it would be preferred to use the underlying call to `rotate::ptr_rotate` in order to avoid the assert in the original implmentations, but it would ideally use this method.

---

<div class="post-metadata">

**Author:** ![tczajka](https://sea2.discourse-cdn.com/flex002/user_avatar/internals.rust-lang.org/tczajka/32/8923_2.png) [@tczajka](https://internals.rust-lang.org/u/tczajka)\
**Post date:** [March 19, 2024, 7:52am UTC](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480/20 "2024-03-19T07:52:06Z")

</div>

> [@scottmcm](#):
>
> If you compute the rotation amount in such a way that you get `-1 as usize`, that will have a jump in the behaviour for any slice length that's not a power of two.

I don't buy this as a counter-argument.

It is very often the case that if you overflow an integer:

- it will panic in debug mode
- you will get wrong answers in release mode

It's not specific to slice rotations. So it is not a convincing argument in favor of making slice rotation try to detect such overflows in its inputs post-factum.

[Next page](https://internals.rust-lang.org/t/remove-panic-from-rotate-left-and-rotate-right/20480.md?page=2)