Amanieu / intrusive-rs

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

Add a get_pointer method to all collections #40

Closed Matthias247 closed 4 years ago

Matthias247 commented 4 years ago

This adds a method which allows to retrieve the pointer that is used to store the element in the collection, and not only the collection.

Fixes #24

Matthias247 commented 4 years ago

24 discussed calling this clone(). That however doesn't work, because Clone is already implemented for Cursor. It could also be called clone_pointer. I here just thought I can keep it in common with .get().

One potential issue: The where A::Pointer : Clone bound might be too weak, because Box<T> also implements it when T: Clone. However in that case the method would not return the actual used pointer, but an unrelated object. I guess if we are not OK with it then a marker trait could be used to fix it, which is implemented for Arc and Rc.

Amanieu commented 4 years ago

I think clone_pointer is a better name here. We want to be clear that we are calling clone, which may be an expensive operation.

Matthias247 commented 4 years ago

I updated the methods to clone_pointer

Matthias247 commented 4 years ago

Shhould we add the following marker trait as a constraint for IntrusivePointer?

/// This is a marker trait which indicates that a pointer is clonable, and
/// that all clones of the pointer will refer to the same underlying object.
/// 
/// This trait enables the `clone_pointer()` methods on intrusive collections.
/// 
/// `Arc` and `Rc` implement this trait by default. In order to allow cloning
/// custom pointers this marker trait needs to be implemented for them.
pub trait SharedPointer: Clone {}

I would be in favor, since allowing to call the method on Box<T: Clone> can really product some weird results.

Amanieu commented 4 years ago

I don't think a SharedPointer trait is necessary: the clone_pointer method will work just as decribed with Box: you'll get a clone of the box, which of course won't point to the same address as the original object.

Amanieu commented 4 years ago

Thanks!