libs-team icon indicating copy to clipboard operation
libs-team copied to clipboard

Create iterator function in std libs: split_item_mut()

Open Norlock opened this issue 1 year ago • 8 comments

Proposal

Problem statement

If I want to use a mutable vector to iterate over the same vector. I can't do this without the borrow checker starts to complain about the vector already being borrowed. There is no user friendly alternative / ergonomic solution for this problem. A possible problem:

Motivating examples or use cases

for particle in particles.iter_mut() {
    for other in particles.iter_mut() {
         if particle != other {
             particle.velocity += other.calculate_gravity(&particle);
         }
    }
}

This is something the borrow checker won't allow. Of course there are options to fix this like updating the index in the vector immediately or using something like swap_remove, both solutions are not very ergonomic / user friendly in my opinion.

Solution sketch

A possible solution is:

pub trait Splitting<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut[T], &mut T, &mut [T]);
}

impl<T> Splitting<T> for Vec<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut T, OtherIterMut<T>) {
        assert!(idx < self.len());

        let (head, rest) = self.split_at_mut(idx);

        let (item, tail) = rest.split_first_mut().unwrap();

        (head, item, tail)
    }
}

And to use it:

let (head, item, tail) = some_vector.split_item_mut(some_index);

// Or chain...
for other in head.iter_mut() {
     other.some_field = item.some_field;
}

for other in tail.iter_mut() {
     other.some_field = item.some_field;
}

Of course this isn't a boundry safe API so you can think of returning the "item" in the tuple as an Option<T>.

Alternatives

pub type OtherIterMut<'a, T> = std::iter::Chain<IterMut<'a, T>, IterMut<'a, T>>;

pub trait Splitting<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut T, OtherIterMut<T>);
}

impl<T> Splitting<T> for Vec<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut T, OtherIterMut<T>) {
        assert!(idx < self.len());

        let (head, rest) = self.split_at_mut(idx);

        let (item, tail) = rest.split_first_mut().unwrap();

        let others = head.iter_mut().chain(tail.iter_mut());

        (item, others)
    }
}

And to use it:

let (item, others) = some_vector.split_item_mut(some_index);
for other in others {
     other.some_field = item.some_field;
}

Links and related work

Also posted on: https://internals.rust-lang.org/t/create-iterator-function-in-std-libs-split-item-mut/19880

Norlock avatar Nov 16 '23 18:11 Norlock

If I want to use a mutable vector to iterate over the same vector (log n).

Doesn't look like log(n) to me at all. It's quadratic.

the8472 avatar Nov 16 '23 18:11 the8472

impl<T: std::fmt::Debug> Splitting<T> for Vec<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut T, OtherIterMut<T>) {

Debug shouldn't be required

programmerjake avatar Nov 16 '23 19:11 programmerjake

impl<T: std::fmt::Debug> Splitting<T> for Vec<T> {
    fn split_item_mut(&mut self, idx: usize) -> (&mut T, OtherIterMut<T>) {

Debug shouldn't be required

It shouldn't, I copied it from my code and forgot to remove. Anyway outside of some mistakes I made I think the idea is useful.

Norlock avatar Nov 16 '23 19:11 Norlock

Does this make sense for many iterators? It seems more like a new variant of the slice-splitting methods, if it returned (&mut [T], &mut T, &mut [T]) as mentioned in alternatives.

cuviper avatar Nov 16 '23 19:11 cuviper

Yes I think that's also fine, but I think in most use cases (my guess) the first variant is more direct. But it doesn't allow multiple iterators so the alternative is maybe better

Norlock avatar Nov 16 '23 20:11 Norlock

As I said in https://internals.rust-lang.org/t/create-iterator-function-in-std-libs-split-item-mut/19880/2?u=scottmcm, this shouldn't return Chain, and definitely shouldn't include a type alias.

I agree with cuviper that this is a slice-to-slices split thing, not something that should have anything to do with iterators.

scottmcm avatar Nov 16 '23 22:11 scottmcm

Ok I updated the issue and swapped the main solution with the alternative one.

Norlock avatar Nov 16 '23 22:11 Norlock

I'll also throw my support behind this on the condition that it is a slice split function rather than some sort of iterator related function.

I've written a function similar to this several times in my own code because I wanted the slice before, the reference to the value at an index, and the slice after.

My mind was pulled particularly to an exercise I wrote when I was learning rust, this part of a quicksort implementation:

    let (lower_portion, pivot_plus_upper) = slice.split_at_mut(pivot);
    let (_pivot_portion, upper_portion) = pivot_plus_upper.split_at_mut(1);
    quicksort_subproblem(lower_portion);
    quicksort_subproblem(upper_portion);

Anyways, the way I envision the API is more like this:

pub fn split_around_mut(&mut self) -> Option<(&mut [T], &mut T, &mut [T])>

pub fn split_around(&self) -> Option<(&[T], &T, &[T])>

It must return None for the case of an empty slice.

ajwock avatar Feb 20 '24 13:02 ajwock