Skip to content
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

Tracking Issue for linked_list_remove #69210

Open
BijanT opened this issue Feb 16, 2020 · 9 comments
Open

Tracking Issue for linked_list_remove #69210

BijanT opened this issue Feb 16, 2020 · 9 comments
Labels
A-collections Area: std::collections. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@BijanT
Copy link
Contributor

BijanT commented Feb 16, 2020

#68705 adds a method in LinkedList to remove an element at a specified index.

The feature gate for the issue is #![feature(linked_list_remove)].

@BijanT BijanT added the C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. label Feb 16, 2020
@Mark-Simulacrum Mark-Simulacrum added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 16, 2020
@jonas-schievink jonas-schievink added the A-collections Area: std::collections. label Feb 16, 2020
@MightyPork
Copy link

What would be a good replacement for this before (if ever) it is stabilized? I mean, other than not using LinkedList.

I tried my usual trick of copying the unstable code to a trait, but the cursors API is unstable as well, so it's not possible here.

pub trait LLRemove<T> {
    fn stabilized_remove(&mut self, at: usize) -> T;
}

impl<T> LLRemove<T> for LinkedList<T>  {
    fn stabilized_remove(&mut self, at: usize) -> T {
        // ...
    }
}

@BijanT
Copy link
Contributor Author

BijanT commented Jun 23, 2020

What I did before adding LinkedList::remove was something like the following:

let split_list = original_list.split_off(index_to_remove);
split_list.pop_front();
original_list.append(split_list);

This seems a little silly to me, which is why I wrote the remove function, but it's stable and get's the job done. I hope this helps.

On another note, if someone from the rust team comes across this, what does need to happen before this feature is stabilized?

@KodrAus KodrAus added I-nominated Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@neeldug
Copy link

neeldug commented Sep 4, 2020

Hi! Just wondering if there could be an implementation to remove an element from a linked list using a reference to the element for faster removal times? (I.e. O(1))

@BijanT
Copy link
Contributor Author

BijanT commented Sep 4, 2020

That's a good idea, but I don't think it's possible with the way the LinkedList struct is set up.
LinkedList is defined as:

pub struct LinkedList<T> {
    head: Option<NonNull<Node<T>>>,
    tail: Option<NonNull<Node<T>>>,
    len: usize,
    marker: PhantomData<Box<Node<T>>>,
}

struct Node<T> {
    next: Option<NonNull<Node<T>>>,
    prev: Option<NonNull<Node<T>>>,
    element: T,
}

With just a reference to the element alone, you would not have access to the next and prev references, so you wouldn't be able to remove it from the list in constant time. You could do it if you had a reference to the Node<T> of the element you want to remove, but that struct is not public.

@neeldug
Copy link

neeldug commented Sep 4, 2020

Right I understand you, but surely making Node<T> public wouldn't be too much of an issue if it provides with such a huge speedup in removal times. Like for instance, when creating any cache using a linked list, surely having O(1) would outweigh the API clutter, having access to nodes seems necessary in my opinion. This would require a method to get a Node rather than an element, which also wouldn't be too much hassle.

@webmaster128
Copy link

Really love the idea of making remove faster (for building caches), but I wonder to what degree this is against the nature of linked lists.

This would require a method to get a Node rather than an element, which also wouldn't be too much hassle.

Where would this node come from? If we have a fn get(at: usize) -> Node, then we still need to iterate over the whole list in order to get the Node. Or create some additional lookup table.

@neeldug
Copy link

neeldug commented Nov 3, 2020

I don't think that it really is against the nature of linked lists, as they are supposed to have guaranteed O(1) insertions and erasures. Quoting from the C++ STL:

(Linked) Lists are sequence containers that allow constant time insert and erase operations anywhere within the sequence, and iteration in both directions.

Where would this node come from? If we have a fn get(at: usize) -> Node, then we still need to iterate over the whole list in order to get the Node. Or create some additional lookup table.

I believe the method to use for this would be a lookup table, however, in C++'s STL they accept a pointer to a node for erasure from the data structure. I believe this could be avoided perhaps by using a lookup table to resolve pointers.

@ImUrX
Copy link

ImUrX commented Mar 12, 2022

No news on an API for returning Node<T>?

@wtfrank
Copy link

wtfrank commented Jan 1, 2023

Rather than a lookup table, what about a second iterator function node_iter_mut() that returns an iterator of Node instead of T? Or better maybe, the existing .iter_mut() object IterMut, which has pointers to adjacent nodes in the list, could be extended with a method next_node() which returns a Node structure instead of T.

You could then add a remove() method to the node structure which would remove it from the linked list in O(1) time rather than O(n).

edit: looking into this some more, the cursor proposal #58533 basically does what I've proposed in this comment by creating a cursor object (similar to an iterator) which has a pop() method which you can use to remove an item from a list with O(1).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-collections Area: std::collections. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants