4

I am working with a LinkedList and I want to remove all elements which do not pass a test. However, I am running into the error cannot move out of borrowed content.

From what I understand, this is because I am working with &mut self, so I do not have the right to invalidate (i.e. move) one of the contained values even for a moment to construct a new list of its values.

In C++/Java, I would simply iterate the list and remove any elements which match a criteria. As there is no remove that I have yet found, I have interpreted it as an iterate, filter, and collect.

The goal is to avoid creating a temporary list, cloning values, and needing take self and return a "new" object. I have constructed an example which produces the same error. Playground.

use std::collections::LinkedList;

#[derive(Debug)]
struct Example {
    list: LinkedList<i8>,
    // Other stuff here
}

impl Example {
    pub fn default() -> Example {
        let mut list = LinkedList::new();
        list.push_back(-5);
        list.push_back(3);
        list.push_back(-1);
        list.push_back(6);
        Example { list }
    }

    // Simmilar idea, but with creating a new list
    pub fn get_positive(&self) -> LinkedList<i8> {
        self.list.iter()
            .filter(|&&x| x > 0)
            .map(|x| x.clone())
            .collect()
    }

    // Now, attempt to filter the elements without cloning anything
    pub fn remove_negative(&mut self) {
        self.list = self.list.into_iter()
            .filter(|&x| x > 0)
            .collect()
    }
}

fn main() {
    let mut e = Example::default();
    println!("{:?}", e.get_positive());
    println!("{:?}", e);
}

In my actual case, I cannot simply consume the wrapping object because it needs to be referenced from different places and contains other important values.

In my research, I found some unsafe code which leads me to question if a safe function could be constructed to perform this action in a similar way to std::mem::replace.

Eadword
  • 160
  • 1
  • 11
  • 1
    *if it were turned into a generic (safe) function* — no such generic safe function is remotely possible. Since that code originally came from Stack Overflow, I updated your link; it now points to the version of the code that is littered with warnings about under exactly which conditions it is valid. – Shepmaster Oct 28 '17 at 22:16
  • 2
    Your text seem to talk about your real world example while your code is just a (more or less) minimal example (which is good!). But maybe you can edit your text to match your code example. This means: removing the mentions of listeners and weak pointer. If I correctly understand your question, it's basically "How can I remove specific elements of a linked list while iterating over it without creating a new list?", right? – Lukas Kalbertodt Oct 28 '17 at 22:17
  • 1
    @LukasKalbertodt That's pretty much it, I included the extra information with the intent of explaining why I could not simply consume `self` and why I wanted to avoid cloning (and later realized cloning was not terrible). Will simplify... – Eadword Oct 28 '17 at 22:20
  • 1
    As the `LinkedList` documentation itself states: *Almost always it is better to use `Vec` or `VecDeque` instead of `LinkedList`* and `Vec` [has a method to do exactly what you want](https://stackoverflow.com/q/37792471/155423). Now we have to figure out how to do the same with a `LinkedList`. – Shepmaster Oct 28 '17 at 22:20
  • @Shepmaster Honestly, you probably are right, I chose a LinkedList because it never needs to be index-wise accessed, and would periodically be appended to and filtered. – Eadword Oct 28 '17 at 22:25
  • At this point, it seems clear that `Vec` is clearly a better choice, and as was pointed out, the issue may be primary a result of an under-developed `LinkedList` API in Rust. https://github.com/rust-lang/rust/issues/39148 – Eadword Oct 30 '17 at 17:01

2 Answers2

3

You can std::mem::swap your field with a temp, and then replace it with your modified list like this. The big downside is the creation of the new LinkedList. I don't know how expensive that is.

pub fn remove_negative(&mut self) {
    let mut temp = LinkedList::new();
    std::mem::swap(&mut temp, &mut self.list);

    self.list = temp.into_iter()
         .filter(|&x| x > 0)
         .collect();
}
hwiechers
  • 14,583
  • 8
  • 53
  • 62
  • 1
    Note that Rust's built-in collections normally have very cheap empty states, and [this holds for `LinkedList`](https://doc.rust-lang.org/src/alloc/linked_list.rs.html#246). – Veedrac Oct 29 '17 at 12:52
  • Yes that would work, and yes it would not be too expensive, but I was trying to avoid it because logically it seems stupid. For example, we have the `retain` function for `Vec` which implements exactly the desired functionality, and in another language such as C++/Java, you could simply remove the elements while iterating. – Eadword Oct 30 '17 at 16:54
  • That said, I think this may be the best solution at present given the Rust API short of switching to a `Vec`. – Eadword Oct 30 '17 at 17:03
0

If the goal is not clone you may use a reference-counting pointer: the clone method on Rc increments the reference counter.

use std::collections::LinkedList;
use std::rc::Rc;

#[derive(Debug)]
struct Example {
    list: LinkedList<Rc<i8>>,
    // ...
}

impl Example {
    pub fn default() -> Example {
        let mut list = LinkedList::new();
        list.push_back(Rc::new(-5));
        list.push_back(Rc::new(3));
        list.push_back(Rc::new(-1));
        list.push_back(Rc::new(6));
        Example { list }
    }

    // Simmilar idea, but with creating a new list
    pub fn get_positive(&self) -> LinkedList<Rc<i8>> {
        self.list.iter()
            .filter(|&x| x.as_ref() > &0)
            .map(|x| x.clone())
            .collect()
    }

    // Now, attempt to filter the elements without cloning anything
    pub fn remove_negative(&mut self) {
        self.list = self.list.iter()
            .filter(|&x| x.as_ref() > &0)
            .map(|x| x.clone())
            .collect()
    }


}


fn main() {
    let mut e = Example::default();
    e.remove_negative();
    println!("{:?}", e.get_positive());
    println!("{:?}", e);
}
attdona
  • 17,196
  • 7
  • 49
  • 60
  • While cloning is sort of an option in my case, I am actually using Arc, so it would now add an additional synchronization overhead. – Eadword Oct 30 '17 at 16:58