-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Completely rewrote the list lib #11453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Better: -> No use of the @pointers anymore -> Has methods instead of functions -> Fully tested and documented New: -> Provides copying/referencing/moving iterators, which implement the Iterator trait (which wasn't the case before. This allows the use of the all "free" methods which come with this trait, as well as the 'for in' syntax. -> Implements FromIterator trait, which allows this collection to play nice with collect() and similar. -> Has a prepend method. (Furthermore, it is efficiently implemented: no copying) -> Has an insert method -> Has methods to access an element by its index (either copying -at() or []-, referencing -at_ref()- or mutably referencing -at_ref_mut()-) -> Generally tries to play with the standard Traits instead of re-inventing the wheel. Good: -> No use at all of unsafe code, or unsafe pointers. TODO: -> At the moment, the FromIterator implementation is naive, and uses repeated append()s which will first go through the whole list. I tried keeping a ref to the last Cell (which should be easy since append() returns a &mut to the last cell), but lifetime management defeated me... -> Add a MutRef iterator. I started working on this, but can't figure out where I'm going wrong or how to side-step the issue (see http://stackoverflow.com/questions/20993909/borrow-vs-mutable-borrow-strange-failure-in-lifetimes)
You can therefore | ||
|
||
*/ | ||
pub fn mov_iter(mut ~self) -> LinkedListMoveIterator<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be called move_iter
FWIW, I don't think an owned singly linked list is particularly useful: using shared ownership (like |
I was also going to comment on that; I haven't finished a review, and this is certainly nice code (good job!) but I can't think of a usecase. |
//! A standard, garbage-collected linked list. | ||
/*! | ||
* This is a straight-forward implementation of a linked-list. | ||
* The implementation in extra uses @pointers, and doesn't use the iterator trait, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think sentence is sensible to put in the module documentation (this appears in the documentation generated for libextra). This code is in libextra right now, and the implementation you are comparing against is deleted by this commit.
Also, I think the prevailing trend is to use //!
doc comments, e.g.
//! This is a ...
//! ...
//! ...
You shouldn't have to clone anything, use at_mut_ref(0), and you'll get a mutable ref to the head, see the at_mut_ref test for a demo. As far as I know (I'm still learning rust), this call doesn't copy anything, and allows modification in place. |
I meant so that the programmer is left with "two" lists: one An owned singly-linked list like this is isomorphic to a stack implemented using a vector, and the vector form is far more efficient:
Converting this code to use |
Ok, got it. I kind of wanted to avoid Rc<> since this was written as a way to get more familiar with Rust's pointers, but maybe the lack of RC is better for lower-level code or embedded stuff ? I haven't looked into Rc much, but I thought it caused higher reliance on the runtime ? This kind of singly linked list still has faster insertions than plain old vector right ? |
…opying iterator as LinkedListCopyIterator, created by calling copy_iter() on a list
@TisButMe: No, a vector has significantly faster insertion than a linked list. When the capacity is full, it doubles the size. I don't think a non-intrusive singly-linked list has a use case, and I think it should just be dropped from the standard library. |
Alright :) Please close the PR, then. |
Thanks for the PR! All contribution is very valued :) |
…lyxyas New lint `clippy::join_absolute_paths` Hey `@ofeeg,` this PR is a copy of all the changes you did in rust-lang/rust-clippy@47171d3 from PR rust-lang/rust-clippy#11208. I wasn't sure how to fix the git history. Hope you're okay with me figuring this out in this separate PR --- changelog: New lint [`join_absolute_paths`] [rust-lang#11453](rust-lang/rust-clippy#11453) Closes: rust-lang/rust-clippy#10655 r? `@Centri3` Since you also gave feedback on the other PR. I hope that I copied everything correctly, but a third pair of eyes would be appreciated :D
Better:
-> No use of the @pointers anymore
-> Has methods instead of functions
-> Fully tested and documented
New:
-> Provides copying/referencing/moving iterators, which implement the Iterator trait (which wasn't the case before. This allows the use of the all "free" methods which come with this trait, as well as the 'for in' syntax.
-> Implements FromIterator trait, which allows this collection to play nice with collect() and similar.
-> Has a prepend method. (Furthermore, it is efficiently implemented: no copying)
-> Has an insert method
-> Has methods to access an element by its index (either copying -at() or []-, referencing -at_ref()- or mutably referencing -at_ref_mut()-)
-> Generally tries to play with the standard Traits instead of re-inventing the wheel.
Good:
-> No use at all of unsafe code, or unsafe pointers.
TODO:
-> At the moment, the FromIterator implementation is naive, and uses repeated append()s which will first go through the whole list. I tried keeping a ref to the last Cell (which should be easy since append() returns a &mut to the last cell), but lifetime management defeated me...
-> Add a MutRef iterator. I started working on this, but can't figure out where I'm going wrong or how to side-step the issue (see http://stackoverflow.com/questions/20993909/borrow-vs-mutable-borrow-strange-failure-in-lifetimes)