rust-lang / libs-team

The home of the library team
Apache License 2.0
115 stars 18 forks source link

Create iterator function in std libs: split_item_mut() #295

Open Norlock opened 9 months ago

Norlock commented 9 months ago

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.

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

the8472 commented 9 months ago

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.

programmerjake commented 9 months ago
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

Norlock commented 9 months ago
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.

cuviper commented 9 months ago

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.

Norlock commented 9 months ago

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

scottmcm commented 9 months ago

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.

Norlock commented 9 months ago

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

ajwock commented 6 months ago

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.