core::slice::Iter and core::slice::IterMut could be replaced with safe code

[HomelikeBrick42]

The title says it, i dont see any reason why the current unsafe implementation using pointers is necessary when it could be written entirely in safe code without too much hassle

use core::{iter::FusedIterator, mem::take};

struct IterMut<'a, T> {
    slice: &'a mut [T],
}

impl<'a, T> Iterator for IterMut<'a, T> {
    type Item = &'a mut T;

    fn next(&mut self) -> Option<Self::Item> {
        let [first, rest @ ..] = take(&mut self.slice) else {
            return None;
        };
        self.slice = rest;
        Some(first)
    }

    fn size_hint(&self) -> (usize, Option<usize>) {
        (self.slice.len(), Some(self.slice.len()))
    }

    fn count(self) -> usize
    where
        Self: Sized,
    {
        self.slice.len()
    }

    fn last(mut self) -> Option<Self::Item>
    where
        Self: Sized,
    {
        self.next_back()
    }

    fn nth(&mut self, n: usize) -> Option<Self::Item> {
        let Some([value, rest @ ..]) = take(&mut self.slice).get_mut(n..) else {
            return None;
        };
        self.slice = rest;
        Some(value)
    }
}

impl<'a, T> DoubleEndedIterator for IterMut<'a, T> {
    fn next_back(&mut self) -> Option<Self::Item> {
        let [rest @ .., last] = take(&mut self.slice) else {
            return None;
        };
        self.slice = rest;
        Some(last)
    }

    fn nth_back(&mut self, n: usize) -> Option<Self::Item> {
        let Some(new_end) = self.slice.len().checked_sub(n) else {
            self.slice = &mut [];
            return None;
        };

        let Some([rest @ .., last]) = take(&mut self.slice).get_mut(..new_end) else {
            return None;
        };
        self.slice = rest;
        Some(last)
    }
}

impl<'a, T> FusedIterator for IterMut<'a, T> {}
impl<'a, T> ExactSizeIterator for IterMut<'a, T> {}

[workingjubilee]

@HomelikeBrick42 You are welcome to PR your version and we will run it through rustc-perf.

[the8472]

Bumping a slice requires both incrementing the pointer and decrementing the length. The current approach only needs to increment one.

Yes, the current design is complex and full of unsafe, but each optimization has been justified by benchmarks and examining llvm-ir/assembly (e.g. #113344, #106343). Occasionally one of the optimizations becomes unnecessary as LLVM gets cleverer, but I wouldn't expect all of them being obsolete. As jubilee says, benchmarks welcome. And assembly comparisons too.

We generally wouldn't recommend that users write code like that when it can be avoided, but slices and their iterators are very central to Rust so going further than usual tends to be worth it.

[scottmcm]

Another place this came up recently: https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/Communicating.20same-provenance.20to.20LLVM/near/426064260

For ZSTs the current implemenation is actually equivalent to what you wrote, just going through extra steps. (I wish it could just be a union, but that pessimizes it in other ways, thus the tricks you see today.)

[scottmcm]

We generally wouldn't recommend that users write code like that when it can be avoided, but slices and their iterators are very central to Rust so going further than usual tends to be worth it.

I wanted to repeat this because it's such an important point.

Slice iterators are the hottest code in all of Rust, both for compile-time and for run-time. Everyone uses them all over the place, and in most code the densest, most perf-critical parts are loops over slices.

Tiny μoptimizations on them are thus worth it in a way that wouldn't be the case in any other rust code.

I'm therefore going to close this, because I don't think it's realistically actionable. I'd of course like to have them be entirely safe, and agree with the8472 that most code should prefer a safe version for something like this, but because there are material codegen differences with the approaches (most notably the difference in how many values need to be updated on Iterator::next) that can't be done with LLVM optimizations, I can't see the slice iterators actually changing to a safe implementation.