Amanieu / intrusive-rs

Intrusive collections for Rust
Apache License 2.0
412 stars 48 forks source link

Is a Drop for LinkedListLink feasible? #73

Closed cehteh closed 2 years ago

cehteh commented 2 years ago

Ben Kimock pointed me to a bug in my code https://github.com/cehteh/cachedb/pull/3

This clearly was caused by my misunderstanding. Still I am wondering whats the reason that LinkedListLink has no Drop implementation that checks if an Link is linked and if so removes it from the list? Do I miss some ill side effect or is it only for performance?

Amanieu commented 2 years ago

Due to the design of the linked list, it's not possible to do this automatically in Drop. Consider the case where this is the last element in the list: the link has no pointer back to the LinkedList object itself since the list is null-terminated (not circular) so it can't reset the head/tail pointers to null.

The reason why the linked list isn't circular is because Rust doesn't have move constructors like C++: the LinkedList must remain valid even when it is moved and its address changes. In C++ this would be handled in the move constructor which can fixup the linked in the head and tail nodes.

cehteh commented 2 years ago

Bummer. That explains it, thanks for the explanation.

Wondering if it would be possible to design this with List being circular and just being a

struct List{
    links: Link; // head and tail
    ...
}

Ensuring that Links are non movable (they shouldn't anyway [how?]). Then add a Link::swap(&mut self,other: &mut Self) to retain moveability. Then one can construct a moveable List (and Links with some efforts).

Amanieu commented 2 years ago

Rust doesn't have a way to prevent objects from being moved. Except maybe for Pin, but that would make the API of this crate much uglier, so I'm generally against it.

cehteh commented 2 years ago

Yes I figured that out meanwhile. I may give it a shot and implement a tiny/ugly cyclic list for my use case.