Implement Cursor for linked lists. (RFC 2570).

[crlf0710]

cc. #58533 cc. @Gankra

r? @Amanieu

[crlf0710]

Also cc @RalfJung since there's so many, sigh, risky node pointers.

[rust-highfive]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2020-01-11T08:58:14.2752251Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-11T08:58:14.2836394Z ##[command]git config gc.auto 0
2020-01-11T08:58:14.2921207Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-11T08:58:14.2998595Z ##[command]git config --get-all http.proxy
2020-01-11T08:58:14.3155453Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68123/merge:refs/remotes/pull/68123/merge
---
2020-01-11T09:03:12.0502648Z     Checking alloc v0.0.0 (/checkout/src/liballoc)
2020-01-11T09:03:12.3207028Z     Checking rustc-demangle v0.1.16
2020-01-11T09:03:12.6393541Z     Checking panic_abort v0.0.0 (/checkout/src/libpanic_abort)
2020-01-11T09:03:12.8181543Z     Checking backtrace v0.3.40
2020-01-11T09:03:14.3225518Z error: type does not implement `fmt::Debug`; consider adding `#[derive(Debug)]` or a manual implementation
2020-01-11T09:03:14.3229983Z      |
2020-01-11T09:03:14.3229983Z      |
2020-01-11T09:03:14.3230591Z 1069 | / pub struct Cursor<'a, T: 'a> {
2020-01-11T09:03:14.3231149Z 1070 | |     index: usize,
2020-01-11T09:03:14.3231692Z 1071 | |     current: Option<NonNull<Node<T>>>,
2020-01-11T09:03:14.3232206Z 1072 | |     list: &'a LinkedList<T>,
2020-01-11T09:03:14.3233180Z      | |_^
2020-01-11T09:03:14.3233627Z      |
2020-01-11T09:03:14.3234658Z      = note: `-D missing-debug-implementations` implied by `-D warnings`
2020-01-11T09:03:14.3235027Z 
2020-01-11T09:03:14.3235027Z 
2020-01-11T09:03:14.3235664Z error: type does not implement `fmt::Debug`; consider adding `#[derive(Debug)]` or a manual implementation
2020-01-11T09:03:14.3236725Z      |
2020-01-11T09:03:14.3236725Z      |
2020-01-11T09:03:14.3237292Z 1088 | / pub struct CursorMut<'a, T: 'a> {
2020-01-11T09:03:14.3238051Z 1089 | |     index: usize,
2020-01-11T09:03:14.3238618Z 1090 | |     current: Option<NonNull<Node<T>>>,
2020-01-11T09:03:14.3239512Z 1091 | |     list: &'a mut LinkedList<T>,
2020-01-11T09:03:14.3240877Z      | |_^
2020-01-11T09:03:14.3241079Z 
2020-01-11T09:03:14.3742229Z error: aborting due to 2 previous errors
2020-01-11T09:03:14.3742668Z 
---
2020-01-11T09:03:14.3955112Z   local time: Sat Jan 11 09:03:14 UTC 2020
2020-01-11T09:03:14.6799054Z   network time: Sat, 11 Jan 2020 09:03:14 GMT
2020-01-11T09:03:14.6800077Z == end clock drift check ==
2020-01-11T09:03:20.8361279Z 
2020-01-11T09:03:20.8481110Z ##[error]Bash exited with code '1'.
2020-01-11T09:03:20.8514707Z ##[section]Starting: Checkout
2020-01-11T09:03:20.8516420Z ==============================================================================
2020-01-11T09:03:20.8516480Z Task         : Get sources
2020-01-11T09:03:20.8516548Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[rust-highfive]

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

2020-01-11T09:13:53.6211453Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-01-11T09:13:53.6223710Z ##[command]git config gc.auto 0
2020-01-11T09:13:53.6227901Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-01-11T09:13:53.6231025Z ##[command]git config --get-all http.proxy
2020-01-11T09:13:53.6233948Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/68123/merge:refs/remotes/pull/68123/merge
---
2020-01-11T09:18:37.3766405Z     Checking alloc v0.0.0 (/checkout/src/liballoc)
2020-01-11T09:18:37.6567906Z     Checking rustc-demangle v0.1.16
2020-01-11T09:18:37.9798651Z     Checking panic_abort v0.0.0 (/checkout/src/libpanic_abort)
2020-01-11T09:18:38.1471625Z     Checking backtrace v0.3.40
2020-01-11T09:18:39.6720194Z error: type does not implement `fmt::Debug`; consider adding `#[derive(Debug)]` or a manual implementation
2020-01-11T09:18:39.6721989Z      |
2020-01-11T09:18:39.6721989Z      |
2020-01-11T09:18:39.6722334Z 1070 | / pub struct Cursor<'a, T: 'a> {
2020-01-11T09:18:39.6731542Z 1071 | |     index: usize,
2020-01-11T09:18:39.6732892Z 1072 | |     current: Option<NonNull<Node<T>>>,
2020-01-11T09:18:39.6733509Z 1073 | |     list: &'a LinkedList<T>,
2020-01-11T09:18:39.6734580Z      | |_^
2020-01-11T09:18:39.6735145Z      |
2020-01-11T09:18:39.6735814Z      = note: `-D missing-debug-implementations` implied by `-D warnings`
2020-01-11T09:18:39.6736022Z 
2020-01-11T09:18:39.6736022Z 
2020-01-11T09:18:39.6736543Z error: type does not implement `fmt::Debug`; consider adding `#[derive(Debug)]` or a manual implementation
2020-01-11T09:18:39.6737467Z      |
2020-01-11T09:18:39.6737467Z      |
2020-01-11T09:18:39.6738023Z 1089 | / pub struct CursorMut<'a, T: 'a> {
2020-01-11T09:18:39.6738541Z 1090 | |     index: usize,
2020-01-11T09:18:39.6739411Z 1091 | |     current: Option<NonNull<Node<T>>>,
2020-01-11T09:18:39.6740103Z 1092 | |     list: &'a mut LinkedList<T>,
2020-01-11T09:18:39.6741265Z      | |_^
2020-01-11T09:18:39.6741443Z 
2020-01-11T09:18:39.7230934Z error: aborting due to 2 previous errors
2020-01-11T09:18:39.7231050Z 
---
2020-01-11T09:18:39.7473865Z   local time: Sat Jan 11 09:18:39 UTC 2020
2020-01-11T09:18:39.9011440Z   network time: Sat, 11 Jan 2020 09:18:39 GMT
2020-01-11T09:18:39.9011518Z == end clock drift check ==
2020-01-11T09:18:40.8006471Z 
2020-01-11T09:18:40.8112786Z ##[error]Bash exited with code '1'.
2020-01-11T09:18:40.8169056Z ##[section]Starting: Checkout
2020-01-11T09:18:40.8170846Z ==============================================================================
2020-01-11T09:18:40.8170897Z Task         : Get sources
2020-01-11T09:18:40.8170938Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[RalfJung]

I'm afraid I am entirely unfamiliar with this code and will not be able to help much with review here.

You could try running the tests in Miri using the setup at https://github.com/RalfJung/miri-test-libstd; let me know if you need any assistance with that.

[Amanieu]

In the RFC thread, I suggested some better method names. Unfortunately this didn't seem to make it into the RFC text itself. Can you rename your code to use these new names?

I've added some initial comments on the implementation, but I'll probably have to do another round of review after you fix those. In particular, the doc comments are pretty bare and are missing a lot of information regarding the exact behavior of the methods in every situation.

[crlf0710]

@Amanieu thank you for the review! I also find an inconsistency when implementing. LinkedList::append takes the other list by mutable reference, while the insert_list(will become splice_after) takes the other list by value. Should i change it to take a mutable reference or leave it as is?

[Amanieu]

I feel that this is a mistake in the design of append. It really should have taken the list by value.

[crlf0710]

Updated the implementation according to the review comments.

[Amanieu]

Nice work!

For the documentation, feel from to copy the doc comments from my intrusive-collections crate.

[Amanieu]

One additional feature you might want to add is an index method which returns Option<usize>. It's free since we need to track the index for splitting anyways. The Debug implementation should be changed to use Option<usize> for the index so that it shows None when pointing to the ghost element.

cc @LukasKalbertodt to update the RFC with this suggestion

[crlf0710]

Updated the implementation according to discussions in rust-lang/rfcs#2847

[Amanieu]

Looks good! The documentation will need more work (especially examples) before this can be stabilized, but the implementation is fine.

@bors r+

[bors]

📌 Commit 06b9a73 has been approved by Amanieu

[zbraniecki]

Has it been considered to add ability to insert/remove elements at cursor? That seems like a core feature to avoid costly inserts and removals at index calculated on the list instance.

[Amanieu]

CursorMut has insert_before, insert_after, remove_current and a bunch of other methods for this.