0

I am making a singly-linked list. When you delete a node, the previous node's next should become the current node's next (prev->next = curr->next;) and return data if the index matches. Otherwise, the previous node becomes the current node and the current node becomes the next node (prev = curr; curr = curr->next;):

struct Node<T> {
    data: T,
    next: Option<Box<Node<T>>>,
}

struct LinkedList<T> {
    head: Option<Box<Node<T>>>,
}

impl LinkedList<i64> {
    fn remove(&mut self, index: usize) -> i64 {
        if self.len() == 0 {
            panic!("LinkedList is empty!");
        }
        if index >= self.len() {
            panic!("Index out of range: {}", index);
        }
        let mut count = 0;
        let mut head = &self.head;
        let mut prev: Option<Box<Node<i64>>> = None;
        loop {
            match head {
                None => {
                    panic!("LinkedList is empty!");
                }
                Some(c) => {
                    // I have borrowed here
                    if count == index {
                        match prev {
                            Some(ref p) => {
                                p.next = c.next;
                                //       ^ cannot move out of borrowed content
                            }
                            _ => continue,
                        }
                        return c.data;
                    } else {
                        count += 1;
                        head = &c.next;
                        prev = Some(*c);
                        //          ^^ cannot move out of borrowed content
                    }
                }
            }
        }
    }

    fn len(&self) -> usize {
        unimplemented!()
    }
}

fn main() {}
error[E0594]: cannot assign to field `p.next` of immutable binding
  --> src/main.rs:31:33
   |
30 |                             Some(ref p) => {
   |                                  ----- consider changing this to `ref mut p`
31 |                                 p.next = c.next;
   |                                 ^^^^^^^^^^^^^^^ cannot mutably borrow field of immutable binding

error[E0507]: cannot move out of borrowed content
  --> src/main.rs:31:42
   |
31 |                                 p.next = c.next;
   |                                          ^ cannot move out of borrowed content

error[E0507]: cannot move out of borrowed content
  --> src/main.rs:40:37
   |
40 |                         prev = Some(*c);
   |                                     ^^ cannot move out of borrowed content

Playground Link for more info.

How can I do this? Is my approach wrong?

Ashniu123
  • 169
  • 1
  • 11
  • Why do you use `impl LinkedList` instead of `T`? – hellow Jul 02 '18 at 11:04
  • Even better: provide a playground link to play with for us :) – hellow Jul 02 '18 at 11:05
  • Possible duplicate: https://stackoverflow.com/questions/41653148/singly-linked-list-in-rust – hellow Jul 02 '18 at 11:10
  • 1
    Also please see: http://cglab.ca/~abeinges/blah/too-many-lists/book/ to get some inspiration – hellow Jul 02 '18 at 11:11
  • 1
    Possible duplicate of [Singly-Linked List in Rust](https://stackoverflow.com/questions/41653148/singly-linked-list-in-rust) – Tim Diekmann Jul 02 '18 at 11:43
  • 1
    I don't think "Singly-Linked List in Rust" is the best duplicate because it doesn't cover OPs error message. There are plenty of other questions that *do* explain that error though... – Shepmaster Jul 02 '18 at 15:45
  • I am pretty sure it isn't a duplicate. As for @Shepmaster, I couldn't find an answer which helps me solve the borrow problem when used in a loop. – Ashniu123 Jul 02 '18 at 16:00

1 Answers1

3

Before you start, go read Learning Rust With Entirely Too Many Linked Lists. People think that linked lists are easy because they've been taught them in languages that either don't care if you introduce memory unsafety or completely take away that agency from the programmer.

Rust does neither, which means that you have to think about things you might never have thought of before.


There are a number of issues with your code. The one that you ask about, "cannot move out of borrowed content" is already well-covered by numerous other questions, so there's no reason to restate all those good answers:

TL;DR: You are attempting to move ownership of next from out of a reference; you cannot.

p.next = c.next;

You are attempting to modify an immutable reference:

let mut head = &self.head;

You allow for people to remove one past the end, which doesn't make sense to me:

if index >= self.len()

You iterate the entire tree not once, but twice before iterating it again to perform the removal:

if self.len() == 0
if index >= self.len()

All of that pales in comparison to the fact that your algorithm is flawed in the eyes of Rust because you attempt to introduce mutable aliasing. If your code were able to compile, you'd have a mutable reference to previous as well as a mutable reference to current. However, you can get a mutable reference to current from previous. This would allow you to break Rust's memory safety guarantees!

Instead, you can only keep track of current and, when the right index is found, break it apart and move the pieces:

fn remove(&mut self, index: usize) -> T {
    self.remove_x(index)
        .unwrap_or_else(|| panic!("index {} out of range", index))
}

fn remove_x(&mut self, mut index: usize) -> Option<T> {
    let mut head = &mut self.head;

    while index > 0 {
        head = match { head }.as_mut() {
            Some(n) => &mut n.next,
            None => return None,
        };
        index -= 1;
    }

    match head.take().map(|x| *x) {
        Some(Node { data, next }) => {
            *head = next;
            Some(data)
        }
        None => None,
    }
}

See also:


Playground Link for more info.

There are numerous problems with the rest of your code, such as the fact that the result of your insert method is unlike any I've ever seen before.

How I'd write it.

Shepmaster
  • 274,917
  • 47
  • 731
  • 969