From a981be75cc45d0640acf1c86fc17166e8bdf953e Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Tue, 29 Jun 2021 15:24:01 +0200 Subject: [PATCH 1/3] add head/tail methods to linked list mutable cursor --- library/alloc/src/collections/linked_list.rs | 122 ++++++++++++++++++ .../src/collections/linked_list/tests.rs | 32 +++++ 2 files changed, 154 insertions(+) diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index 1a58ad51f78d3..a21eedf02f100 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -1506,6 +1506,128 @@ impl<'a, T> CursorMut<'a, T> { self.index = 0; unsafe { self.list.split_off_before_node(self.current, split_off_idx) } } + + /// Appends an element to the front of the cursor's parent list. The node + /// that the cursor points to is unchanged, even if it is the "ghost" node. + /// + /// This operation should compute in O(1) time. + // `push_front` continues to point to "ghost" when it addes a node to mimic + // the behavior of `insert_before` on an empty list. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn push_front(&mut self, elt: T) { + // Safety: We know that `push_front` does not change the position in + // memory of other nodes. This ensures that `self.current` remains + // valid. + self.list.push_front(elt); + self.index += 1; + } + + /// Appends an element to the back of the cursor's parent list. The node + /// that the cursor points to is unchanged, even if it is the "ghost" node. + /// + /// This operation should compute in O(1) time. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn push_back(&mut self, elt: T) { + // Safety: We know that `push_back` does not change the position in + // memory of other nodes. This ensures that `self.current` remains + // valid. + self.list.push_back(elt); + } + + /// Removes the first element from the cursor's parent list and returns it, + /// or None if the list is empty. The element the cursor points to remains + /// unchanged, unless it was pointing to the front element. In that case, it + /// points to the new front element. + /// + /// This operation should compute in O(1) time. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn pop_front(&mut self) -> Option { + // We can't check if current is empty, we must check the list directly. + // It is possible for `self.current == None` and the list to be + // non-empty. + if self.list.is_empty() { + None + } else { + // We can't point to the node that we pop. Copying the behavior of + // `remove_current`, we move on the the next node in the sequence. + // If the list is of length 1 then we end pointing to the "ghost" + // node, which is expected. + if self.list.head == self.current { + self.move_next(); + } + // We always need to change the index since `head` comes before any + // other element. + self.index.checked_sub(1).unwrap_or(0); + self.list.pop_front() + } + } + + /// Removes the last element from the cursor's parent list and returns it, + /// or None if the list is empty. The element the cursor points to remains + /// unchanged, unless it was pointing to the back element. In that case, it + /// points to the new back element. + /// + /// This operation should compute in O(1) time. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn pop_back(&mut self) -> Option { + if self.list.is_empty() { + None + } else { + if self.list.tail == self.current { + self.move_prev() + } + // We don't need to change the index since `current` points to a + // node before `tail`. + self.list.pop_back() + } + } + + /// Provides a reference to the front element of the cursor's parent list, + /// or None if the list is empty. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn front(&self) -> Option<&T> { + self.list.front() + } + + /// Provides a mutable reference to the front element of the cursor's + /// parent list, or None if the list is empty. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn front_mut(&mut self) -> Option<&mut T> { + self.list.front_mut() + } + + /// Provides a reference to the back element of the cursor's parent list, + /// or None if the list is empty. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn back(&self) -> Option<&T> { + self.list.back() + } + + /// Provides a mutable reference to back element of the cursor's parent + /// list, or `None` if the list is empty. + /// + /// # Examples + /// Building and mutating a list with a cursor, then getting the back element: + /// ``` + /// #![feature(linked_list_cursors)] + /// use std::collections::LinkedList; + /// let mut dl = LinkedList::new(); + /// dl.push_front(3); + /// dl.push_front(2); + /// dl.push_front(1); + /// let mut cursor = dl.cursor_front_mut(); + /// *cursor.current().unwrap() = 99; + /// *cursor.back_mut().unwrap() = 0; + /// let mut contents = dl.into_iter(); + /// assert_eq!(contents.next(), Some(99)); + /// assert_eq!(contents.next(), Some(2)); + /// assert_eq!(contents.next(), Some(0)); + /// assert_eq!(contents.next(), None); + /// ``` + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn back_mut(&mut self) -> Option<&mut T> { + self.list.back_mut() + } } /// An iterator produced by calling `drain_filter` on LinkedList. diff --git a/library/alloc/src/collections/linked_list/tests.rs b/library/alloc/src/collections/linked_list/tests.rs index ad643a7bdf194..429685260fd77 100644 --- a/library/alloc/src/collections/linked_list/tests.rs +++ b/library/alloc/src/collections/linked_list/tests.rs @@ -428,3 +428,35 @@ fn test_cursor_mut_insert() { check_links(&m); assert_eq!(m.iter().cloned().collect::>(), &[200, 201, 202, 203, 1, 100, 101]); } + +#[test] +fn test_cursor_push_front_back() { + let mut ll: LinkedList = LinkedList::new(); + ll.extend(&[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + let mut c = ll.cursor_front_mut(); + assert_eq!(c.current(), Some(&mut 1)); + assert_eq!(c.index(), Some(0)); + c.push_front(0); + assert_eq!(c.current(), Some(&mut 1)); + assert_eq!(c.peek_prev(), Some(&mut 0)); + assert_eq!(c.index(), Some(1)); + c.push_back(11); + drop(c); + assert_eq!(ll, (0..12).collect()); + check_links(&ll); +} + +#[test] +fn test_cursor_pop_front_back() { + let mut ll: LinkedList = LinkedList::new(); + ll.extend(&[1, 2, 3, 4, 5, 6]); + let mut c = ll.cursor_back_mut(); + assert_eq!(c.pop_front(), Some(1)); + c.move_prev(); + c.move_prev(); + c.move_prev(); + assert_eq!(c.pop_back(), Some(6)); + drop(c); + assert_eq!(ll, (2..6).collect()); + check_links(&ll); +} From e77acf7d27f7690f6cdb18d2ce68a37cbabd884c Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Tue, 29 Jun 2021 15:35:14 +0200 Subject: [PATCH 2/3] Add non-mutable methods to `Cursor` --- library/alloc/src/collections/linked_list.rs | 14 ++++++++++++++ library/alloc/src/collections/linked_list/tests.rs | 3 +++ 2 files changed, 17 insertions(+) diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index a21eedf02f100..4eecf5703a4a1 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -1243,6 +1243,20 @@ impl<'a, T> Cursor<'a, T> { prev.map(|prev| &(*prev.as_ptr()).element) } } + + /// Provides a reference to the front element of the cursor's parent list, + /// or None if the list is empty. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn front(&self) -> Option<&T> { + self.list.front() + } + + /// Provides a reference to the back element of the cursor's parent list, + /// or None if the list is empty. + #[unstable(feature = "linked_list_cursors", issue = "58533")] + pub fn back(&self) -> Option<&T> { + self.list.back() + } } impl<'a, T> CursorMut<'a, T> { diff --git a/library/alloc/src/collections/linked_list/tests.rs b/library/alloc/src/collections/linked_list/tests.rs index 429685260fd77..45349a7df12d3 100644 --- a/library/alloc/src/collections/linked_list/tests.rs +++ b/library/alloc/src/collections/linked_list/tests.rs @@ -456,6 +456,9 @@ fn test_cursor_pop_front_back() { c.move_prev(); c.move_prev(); assert_eq!(c.pop_back(), Some(6)); + let c = c.as_cursor(); + assert_eq!(c.front(), Some(&2)); + assert_eq!(c.back(), Some(&5)); drop(c); assert_eq!(ll, (2..6).collect()); check_links(&ll); From c4ad273fe168cba92a298e35826b37f7177ce599 Mon Sep 17 00:00:00 2001 From: Ian Wahbe Date: Thu, 1 Jul 2021 21:08:01 +0200 Subject: [PATCH 3/3] Implement changes suggested by @Amanieu --- library/alloc/src/collections/linked_list.rs | 27 ++++++++++++------- .../src/collections/linked_list/tests.rs | 12 +++++++++ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/library/alloc/src/collections/linked_list.rs b/library/alloc/src/collections/linked_list.rs index 4eecf5703a4a1..ea216786ea2d4 100644 --- a/library/alloc/src/collections/linked_list.rs +++ b/library/alloc/src/collections/linked_list.rs @@ -1247,14 +1247,14 @@ impl<'a, T> Cursor<'a, T> { /// Provides a reference to the front element of the cursor's parent list, /// or None if the list is empty. #[unstable(feature = "linked_list_cursors", issue = "58533")] - pub fn front(&self) -> Option<&T> { + pub fn front(&self) -> Option<&'a T> { self.list.front() } /// Provides a reference to the back element of the cursor's parent list, /// or None if the list is empty. #[unstable(feature = "linked_list_cursors", issue = "58533")] - pub fn back(&self) -> Option<&T> { + pub fn back(&self) -> Option<&'a T> { self.list.back() } } @@ -1546,6 +1546,11 @@ impl<'a, T> CursorMut<'a, T> { // memory of other nodes. This ensures that `self.current` remains // valid. self.list.push_back(elt); + if self.current().is_none() { + // The index of "ghost" is the length of the list, so we just need + // to increment self.index to reflect the new length of the list. + self.index += 1; + } } /// Removes the first element from the cursor's parent list and returns it, @@ -1565,13 +1570,12 @@ impl<'a, T> CursorMut<'a, T> { // We can't point to the node that we pop. Copying the behavior of // `remove_current`, we move on the the next node in the sequence. // If the list is of length 1 then we end pointing to the "ghost" - // node, which is expected. + // node at index 0, which is expected. if self.list.head == self.current { self.move_next(); + } else { + self.index -= 1; } - // We always need to change the index since `head` comes before any - // other element. - self.index.checked_sub(1).unwrap_or(0); self.list.pop_front() } } @@ -1579,7 +1583,7 @@ impl<'a, T> CursorMut<'a, T> { /// Removes the last element from the cursor's parent list and returns it, /// or None if the list is empty. The element the cursor points to remains /// unchanged, unless it was pointing to the back element. In that case, it - /// points to the new back element. + /// points to the "ghost" element. /// /// This operation should compute in O(1) time. #[unstable(feature = "linked_list_cursors", issue = "58533")] @@ -1588,10 +1592,13 @@ impl<'a, T> CursorMut<'a, T> { None } else { if self.list.tail == self.current { - self.move_prev() + // The index now reflects the length of the list. It was the + // length of the list minus 1, but now the list is 1 smaller. No + // change is needed for `index`. + self.current = None; + } else if self.current.is_none() { + self.index = self.list.len - 1; } - // We don't need to change the index since `current` points to a - // node before `tail`. self.list.pop_back() } } diff --git a/library/alloc/src/collections/linked_list/tests.rs b/library/alloc/src/collections/linked_list/tests.rs index 45349a7df12d3..5a65ed7a962e9 100644 --- a/library/alloc/src/collections/linked_list/tests.rs +++ b/library/alloc/src/collections/linked_list/tests.rs @@ -442,6 +442,8 @@ fn test_cursor_push_front_back() { assert_eq!(c.index(), Some(1)); c.push_back(11); drop(c); + let p = ll.cursor_back().front().unwrap(); + assert_eq!(p, &0); assert_eq!(ll, (0..12).collect()); check_links(&ll); } @@ -459,7 +461,17 @@ fn test_cursor_pop_front_back() { let c = c.as_cursor(); assert_eq!(c.front(), Some(&2)); assert_eq!(c.back(), Some(&5)); + assert_eq!(c.index(), Some(1)); drop(c); assert_eq!(ll, (2..6).collect()); check_links(&ll); + let mut c = ll.cursor_back_mut(); + assert_eq!(c.current(), Some(&mut 5)); + assert_eq!(c.index, 3); + assert_eq!(c.pop_back(), Some(5)); + assert_eq!(c.current(), None); + assert_eq!(c.index, 3); + assert_eq!(c.pop_back(), Some(4)); + assert_eq!(c.current(), None); + assert_eq!(c.index, 2); }