From 498c10a62e5fd156f1ae141cf1c4c5df31932b2a Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 11 Jan 2020 16:53:27 +0100 Subject: [PATCH 1/4] Convert tabs to space in RFC 2570 Linked List Cursors Tabs render as 8 spaces on GitHub which is different from the standard Rust style (4 spaces). --- text/2570-linked-list-cursors.md | 154 +++++++++++++++---------------- 1 file changed, 77 insertions(+), 77 deletions(-) diff --git a/text/2570-linked-list-cursors.md b/text/2570-linked-list-cursors.md index 596e8078c..f2cd3d48a 100644 --- a/text/2570-linked-list-cursors.md +++ b/text/2570-linked-list-cursors.md @@ -61,21 +61,21 @@ list many times. With the cursor interface, one can do the following: ``` rust fn remove_replace(list: &mut LinkedList, p: P, f: F) - where P: Fn(&T) -> bool, F: Fn(T) -> T + where P: Fn(&T) -> bool, F: Fn(T) -> T { - let mut cursor = list.cursor_mut(); - // move to the first element, if it exists - loop { - let should_replace = match cursor.peek() { - Some(element) => p(element), - None => break, - }; - if should_replace { - let old_element = cursor.pop().unwrap(); - cursor.insert(f(old_element)); - } - cursor.move_next(); - } + let mut cursor = list.cursor_mut(); + // move to the first element, if it exists + loop { + let should_replace = match cursor.peek() { + Some(element) => p(element), + None => break, + }; + if should_replace { + let old_element = cursor.pop().unwrap(); + cursor.insert(f(old_element)); + } + cursor.move_next(); + } } ``` @@ -84,31 +84,31 @@ iterator, perform operations on it and collect. This is easier, however it still requires much needless allocation. For another example, consider code that was previously using `IterMut` -extensions. +extensions. ``` rust fn main() { - let mut list: LinkedList<_> = (0..10).collect(); - let mut iter = list.iter_mut(); - while let Some(x) = iter.next() { - if x >= 5 { - break; - } - } - iter.insert_next(12); + let mut list: LinkedList<_> = (0..10).collect(); + let mut iter = list.iter_mut(); + while let Some(x) = iter.next() { + if x >= 5 { + break; + } + } + iter.insert_next(12); } ``` This can be changed almost verbatim to `CursorMut`: ``` rust fn main() { - let mut list: LinkedList<_> = (0..10).collect(); - let mut cursor = list.cursor_mut() { - while let Some(x) = cursor.peek_next() { - if x >= 5 { - break; - } - cursor.move_next(); - } - cursor.insert(12); + let mut list: LinkedList<_> = (0..10).collect(); + let mut cursor = list.cursor_mut() { + while let Some(x) = cursor.peek_next() { + if x >= 5 { + break; + } + cursor.move_next(); + } + cursor.insert(12); } ``` In general, the cursor interface is not the easiest way to do something. @@ -133,61 +133,61 @@ These would provide the following interface: ``` rust impl<'list, T> Cursor<'list, T> { - /// Move to the subsequent element of the list if it exists or the empty - /// element - pub fn move_next(&mut self); - /// Move to the previous element of the list - pub fn move_prev(&mut self); - - /// Get the current element - pub fn current(&self) -> Option<&'list T>; - /// Get the following element - pub fn peek(&self) -> Option<&'list T>; - /// Get the previous element - pub fn peek_before(&self) -> Option<&'list T>; + /// Move to the subsequent element of the list if it exists or the empty + /// element + pub fn move_next(&mut self); + /// Move to the previous element of the list + pub fn move_prev(&mut self); + + /// Get the current element + pub fn current(&self) -> Option<&'list T>; + /// Get the following element + pub fn peek(&self) -> Option<&'list T>; + /// Get the previous element + pub fn peek_before(&self) -> Option<&'list T>; } impl<'list T> CursorMut<'list, T> { - /// Move to the subsequent element of the list if it exists or the empty - /// element - pub fn move_next(&mut self); - /// Move to the previous element of the list - pub fn move_prev(&mut self); + /// Move to the subsequent element of the list if it exists or the empty + /// element + pub fn move_next(&mut self); + /// Move to the previous element of the list + pub fn move_prev(&mut self); - /// Get the current element - pub fn current(&mut self) -> Option<&mut T>; - /// Get the next element - pub fn peek(&mut self) -> Option<&mut T>; - /// Get the previous element - pub fn peek_before(&mut self) -> Option<&mut T>; + /// Get the current element + pub fn current(&mut self) -> Option<&mut T>; + /// Get the next element + pub fn peek(&mut self) -> Option<&mut T>; + /// Get the previous element + pub fn peek_before(&mut self) -> Option<&mut T>; - /// Get an immutable cursor at the current element - pub fn as_cursor<'cm>(&'cm self) -> Cursor<'cm, T>; + /// Get an immutable cursor at the current element + pub fn as_cursor<'cm>(&'cm self) -> Cursor<'cm, T>; - // Now the list editing operations + // Now the list editing operations - /// Insert `item` after the cursor - pub fn insert(&mut self, item: T); - /// Insert `item` before the cursor - pub fn insert_before(&mut self, item: T); + /// Insert `item` after the cursor + pub fn insert(&mut self, item: T); + /// Insert `item` before the cursor + pub fn insert_before(&mut self, item: T); - /// Remove and return the item following the cursor - pub fn pop(&mut self) -> Option; - /// Remove and return the item before the cursor - pub fn pop_before(&mut self) -> Option; + /// Remove and return the item following the cursor + pub fn pop(&mut self) -> Option; + /// Remove and return the item before the cursor + pub fn pop_before(&mut self) -> Option; - /// Insert `list` between the current element and the next - pub fn insert_list(&mut self, list: LinkedList); - /// Insert `list` between the previous element and current - pub fn insert_list_before(&mut self, list: LinkedList); + /// Insert `list` between the current element and the next + pub fn insert_list(&mut self, list: LinkedList); + /// Insert `list` between the previous element and current + pub fn insert_list_before(&mut self, list: LinkedList); - /// Split the list in two after the current element - /// The returned list consists of all elements following the current one. - // note: consuming the cursor is not necessary here, but it makes sense - // given the interface - pub fn split(self) -> LinkedList; - /// Split the list in two before the current element - pub fn split_before(self) -> LinkedList; + /// Split the list in two after the current element + /// The returned list consists of all elements following the current one. + // note: consuming the cursor is not necessary here, but it makes sense + // given the interface + pub fn split(self) -> LinkedList; + /// Split the list in two before the current element + pub fn split_before(self) -> LinkedList; } ``` One should closely consider the lifetimes in this interface. Both `Cursor` and From b486793c2e9349faa8bf2dbd9d64ea3d83aeec4e Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 11 Jan 2020 17:18:35 +0100 Subject: [PATCH 2/4] Rename some methods of Linked List Cursor These changes is where everyone seems to agree on. - peek -> peek_next - peek_before -> peek_prev - insert -> insert_after - insert_list -> splice_after - insert_list_before -> splice_before - split -> split_after --- text/2570-linked-list-cursors.md | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/text/2570-linked-list-cursors.md b/text/2570-linked-list-cursors.md index f2cd3d48a..8d53b6337 100644 --- a/text/2570-linked-list-cursors.md +++ b/text/2570-linked-list-cursors.md @@ -66,13 +66,13 @@ fn remove_replace(list: &mut LinkedList, p: P, f: F) let mut cursor = list.cursor_mut(); // move to the first element, if it exists loop { - let should_replace = match cursor.peek() { + let should_replace = match cursor.peek_next() { Some(element) => p(element), None => break, }; if should_replace { let old_element = cursor.pop().unwrap(); - cursor.insert(f(old_element)); + cursor.insert_after(f(old_element)); } cursor.move_next(); } @@ -108,7 +108,7 @@ fn main() { } cursor.move_next(); } - cursor.insert(12); + cursor.insert_after(12); } ``` In general, the cursor interface is not the easiest way to do something. @@ -142,9 +142,9 @@ impl<'list, T> Cursor<'list, T> { /// Get the current element pub fn current(&self) -> Option<&'list T>; /// Get the following element - pub fn peek(&self) -> Option<&'list T>; + pub fn peek_next(&self) -> Option<&'list T>; /// Get the previous element - pub fn peek_before(&self) -> Option<&'list T>; + pub fn peek_prev(&self) -> Option<&'list T>; } impl<'list T> CursorMut<'list, T> { @@ -157,9 +157,9 @@ impl<'list T> CursorMut<'list, T> { /// Get the current element pub fn current(&mut self) -> Option<&mut T>; /// Get the next element - pub fn peek(&mut self) -> Option<&mut T>; + pub fn peek_next(&mut self) -> Option<&mut T>; /// Get the previous element - pub fn peek_before(&mut self) -> Option<&mut T>; + pub fn peek_prev(&mut self) -> Option<&mut T>; /// Get an immutable cursor at the current element pub fn as_cursor<'cm>(&'cm self) -> Cursor<'cm, T>; @@ -167,7 +167,7 @@ impl<'list T> CursorMut<'list, T> { // Now the list editing operations /// Insert `item` after the cursor - pub fn insert(&mut self, item: T); + pub fn insert_after(&mut self, item: T); /// Insert `item` before the cursor pub fn insert_before(&mut self, item: T); @@ -177,15 +177,15 @@ impl<'list T> CursorMut<'list, T> { pub fn pop_before(&mut self) -> Option; /// Insert `list` between the current element and the next - pub fn insert_list(&mut self, list: LinkedList); + pub fn splice_after(&mut self, list: LinkedList); /// Insert `list` between the previous element and current - pub fn insert_list_before(&mut self, list: LinkedList); + pub fn splice_before(&mut self, list: LinkedList); /// Split the list in two after the current element /// The returned list consists of all elements following the current one. // note: consuming the cursor is not necessary here, but it makes sense // given the interface - pub fn split(self) -> LinkedList; + pub fn split_after(self) -> LinkedList; /// Split the list in two before the current element pub fn split_before(self) -> LinkedList; } @@ -195,17 +195,17 @@ One should closely consider the lifetimes in this interface. Both `Cursor` and the annotation of `'list`. The lifetime elision for their constructors is correct as -``` +```rust pub fn cursor(&self) -> Cursor ``` becomes -``` +```rust pub fn cursor<'list>(&'list self) -> Cursor<'list, T> ``` which is what we would expect. (the same goes for `CursorMut`). -Since `Cursor` cannot mutate its list, `current`, `peek` and `peek_before` all -live as long as `'list`. However, in `CursorMut` we must be careful to make +Since `Cursor` cannot mutate its list, `current`, `peek_next` and `peek_prev` +all live as long as `'list`. However, in `CursorMut` we must be careful to make these methods borrow. Otherwise, one could produce multiple mutable references to the same element. From 41a81ea160672d6cf342a3566393d7a6fdd65005 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Sat, 11 Jan 2020 19:11:21 +0100 Subject: [PATCH 3/4] Replace `pop` and `pop_before` with `remove_current` in Linked List Cusors The name "pop" was critized in the discussion thread but despite agreement it should be changed, no one did. This commit implements basically the suggestion by Amanieu (but `remove_current` instead of `remove`). There is some discussion however, over if there should be two methods (differing in what element is the current one after deletion). This should be discussed in the tracking issue before stabilizing the feature. --- text/2570-linked-list-cursors.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/text/2570-linked-list-cursors.md b/text/2570-linked-list-cursors.md index 8d53b6337..f3c11b031 100644 --- a/text/2570-linked-list-cursors.md +++ b/text/2570-linked-list-cursors.md @@ -71,7 +71,7 @@ fn remove_replace(list: &mut LinkedList, p: P, f: F) None => break, }; if should_replace { - let old_element = cursor.pop().unwrap(); + let old_element = cursor.remove_current().unwrap(); cursor.insert_after(f(old_element)); } cursor.move_next(); @@ -171,10 +171,9 @@ impl<'list T> CursorMut<'list, T> { /// Insert `item` before the cursor pub fn insert_before(&mut self, item: T); - /// Remove and return the item following the cursor - pub fn pop(&mut self) -> Option; - /// Remove and return the item before the cursor - pub fn pop_before(&mut self) -> Option; + /// Remove the current item. The new current item is the item following the + /// removed one. + pub fn remove_current(&mut self) -> Option; /// Insert `list` between the current element and the next pub fn splice_after(&mut self, list: LinkedList); From 772fb67c7e38dc57f8629bea1476529c70d40529 Mon Sep 17 00:00:00 2001 From: Lukas Kalbertodt Date: Tue, 14 Jan 2020 16:07:50 +0100 Subject: [PATCH 4/4] Rename a few more methods in Linked List Cursor RFC These are just the names of the initially implementation. --- text/2570-linked-list-cursors.md | 34 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/text/2570-linked-list-cursors.md b/text/2570-linked-list-cursors.md index f3c11b031..cb2f8bf6a 100644 --- a/text/2570-linked-list-cursors.md +++ b/text/2570-linked-list-cursors.md @@ -63,7 +63,7 @@ list many times. With the cursor interface, one can do the following: fn remove_replace(list: &mut LinkedList, p: P, f: F) where P: Fn(&T) -> bool, F: Fn(T) -> T { - let mut cursor = list.cursor_mut(); + let mut cursor = list.cursor_front_mut(); // move to the first element, if it exists loop { let should_replace = match cursor.peek_next() { @@ -101,7 +101,7 @@ This can be changed almost verbatim to `CursorMut`: ``` rust fn main() { let mut list: LinkedList<_> = (0..10).collect(); - let mut cursor = list.cursor_mut() { + let mut cursor = list.cursor_front_mut() { while let Some(x) = cursor.peek_next() { if x >= 5 { break; @@ -122,17 +122,26 @@ One gets a cursor the exact same way as one would get an iterator. The returned cursor would point to the "empty" element, i.e. if you got an element and called `current` you would receive `None`. ``` rust -// Provides a cursor to the first element of the list -pub fn cursor(&self) -> Cursor; +/// Provides a cursor to the first element of the list. +pub fn cursor_front(&self) -> Cursor; -/// Provides a cursor with mutable references and access to the list -pub fn cursor_mut(&mut self) -> CursorMut; +/// Provides a mutable cursor to the first element of the list. +pub fn cursor_front_mut(&mut self) -> CursorMut; + +/// Provides a cursor to the last element of the list. +pub fn cursor_back(&self) -> Cursor; + +/// Provides a mutable cursor to the last element of the list. +pub fn cursor_back_mut(&mut self) -> CursorMut; ``` These would provide the following interface: ``` rust impl<'list, T> Cursor<'list, T> { + /// Returns the cursor position index within the `LinkedList`. + pub fn index(&self) -> Option; + /// Move to the subsequent element of the list if it exists or the empty /// element pub fn move_next(&mut self); @@ -148,6 +157,9 @@ impl<'list, T> Cursor<'list, T> { } impl<'list T> CursorMut<'list, T> { + /// Returns the cursor position index within the `LinkedList`. + pub fn index(&self) -> Option; + /// Move to the subsequent element of the list if it exists or the empty /// element pub fn move_next(&mut self); @@ -182,11 +194,9 @@ impl<'list T> CursorMut<'list, T> { /// Split the list in two after the current element /// The returned list consists of all elements following the current one. - // note: consuming the cursor is not necessary here, but it makes sense - // given the interface - pub fn split_after(self) -> LinkedList; + pub fn split_after(&mut self) -> LinkedList; /// Split the list in two before the current element - pub fn split_before(self) -> LinkedList; + pub fn split_before(&mut self) -> LinkedList; } ``` One should closely consider the lifetimes in this interface. Both `Cursor` and @@ -195,11 +205,11 @@ the annotation of `'list`. The lifetime elision for their constructors is correct as ```rust -pub fn cursor(&self) -> Cursor +pub fn cursor_front(&self) -> Cursor ``` becomes ```rust -pub fn cursor<'list>(&'list self) -> Cursor<'list, T> +pub fn cursor_front<'list>(&'list self) -> Cursor<'list, T> ``` which is what we would expect. (the same goes for `CursorMut`).