Amanieu / intrusive-rs

Intrusive collections for Rust
Apache License 2.0
423 stars 50 forks source link

How to return a mutable reference to one of the collection members? #38

Closed Matthias247 closed 5 years ago

Matthias247 commented 5 years ago

I'm currently trying to use the library to place an element into multiple collections inside the same overall struct.

I would like that struct to have an accessor, which returns me a mutable reference to one of the stored elements. This would be not be able to mutate the collection, since the accessor requires &mut self, therefore everything should be fine from a safety perspective. However I can not get it to run without rustc complaining. Do you have any idea?

This is what I tried:

struct ElementNode {
    tree_link: RBTreeLink,
    inner: RefCell<ElementImpl>,
}

intrusive_adapter!(ElementTreeAdapter = Rc<ElementNode>: ElementNode {
    tree_link: RBTreeLink
});

pub struct ElementManager {
    element_tree: RBTree<ElementTreeAdapter>,
}

impl ElementManager {
    pub fn get_element_mut(&mut self, id: &ElementId) -> Option<&mut ElementImpl> {
        let node = self.element_tree.find_mut(id).get()?;

        Some(&mut (*node.inner.borrow_mut()))
    }
}

And it yields:

80 |         Some(&mut (*node.inner.borrow_mut()))
   |         ^^^^^^^^^^^^-----------------------^^
   |         |           |
   |         |           temporary value created here
   |         returns a value referencing data owned by the current function
Amanieu commented 5 years ago

You can't return a &mut ElementImpl, but you should be able to return a RefMut<ElementImpl>.

Matthias247 commented 5 years ago

Thanks for the quick response!

I tried changing the return value to Option<RefMut<ElementImpl>>, but I still get the error:

   |
78 |         let node = self.element_tree.find_mut(id).get()?;
   |                    ------------------------------ temporary value created here
79 | 
80 |         Some(node.inner.borrow_mut())
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returns a value referencing data owned by the current function

I think it dislikes the Cursor that is created. Although we actually do not require that later on - we just reference the inner data.

Amanieu commented 5 years ago

Oh right, you should use an non-mut cursor since you aren't actually modifiying the tree itself.

Amanieu commented 5 years ago

Basically use find instead of find_mut.

Matthias247 commented 5 years ago

Ah, I just tried it and that one works! Thanks!

Guess I went down the wrong path and looked for mut because I actually wanted to be able to mutate the element. But actually find_mut even doesn't allow that, since one can only call get() and there is no get_mut (which I would have required). Wonder whether any get_mut() could be added without having to use a RefCell. But I guess the answer is no due to other lists having the ability to refer to elements. In my cases they all would be part of the same overall container, but I guess that's guaranteed for everyone.

I was also able to be a bit more general and instead of RefMut return: Option<impl DerefMut<Target=ElementImpl> + 'a>.