Cloning Cell<Option<Rc<A>>>

code review
#1

I have the following structure

struct SceneNode{
    //...
    mesh : Cell<Option<Rc<SceneMesh>>>,
}

Each SceneNode can have an optional reference to a SceneMesh. The SceneNode is part of a hierarchy and there can be many references to the same Node ( which is why I use Cell for the mesh. )

This works but it is quite clunky to use as I cant see a way to use mesh without first doing a take, then clone, then finally put it back in.

let local = scene_node.mesh.take();   // take the mesh from the cell. Cell is now empty
scene_node.mesh.set(local.clone());   // put the cloned copy back in. 
//.. use local

The above works but it feels very, very wrong. Is there a cleaner, or a more succinct way of cloning a Cell<Option<Rc<Struct>>>>

#2

It sounds like you want something more like this.

struct SceneNode {
    mesh: Rc<Cell<Option<SceneMesh>>>,
}

The first thing that looks strange is the Rc in a Cell.

If you want one instance to have many owners, use an Rc. If you want interior mutability, use a Cell. When you put an Rc in a Cell, that means you will be swapping out one Rc for another instance can have many owners. When you put a Cell in an Rc, that means you will be making changes that can be observed by the many owners of that Rc. If you are replacing the content of a Cell, you usually don't need the old contents anymore so there's no reason to use a smart pointer to keep those references alive. To put it another way, Rc<Cell> is a type with many owners and mutable contents. Cell<Rc> is a type that can be mutated to hold different instances of a type that can have many owners.

The second thing that stands out is an option in a cell.

If you want nothing to be there, just wrap the type in an Option so it can be None or Some(Thing), as you've done. However, if SceneMesh has an "empty" or "nil" state without contents then let that be the default by implementing the Default trait. When you call Cell::take it will replace the SceneMesh with whatever Default::default(your_cell) returns. If there is no logical default and it makes more sense for it to be empty, the Option in a Cell is fine.

I hope this addresses what feels wrong about it to you.

1 Like
#3

I don't think you can make this code more right without broad refactoring of the code structure. You're in workaround to the Rust's shared XOR mutable rule using the Cell<T> type. And it's common to feel wrong for workaround code.

It's a cost to have shared mutability in your logic. I'd not say it's bad - sometimes it's really handy and needed. But still it is unusual Rust code, feeling alien would be normal.

#4

There isn't really any better way.

I guess that it would be sound to use unsafe to produce an &T to the contents and clone it, since the Clone impl of Rc does not access the Cell it is contained in.

#5

Via memcpy the raw pointer out, yeah it can be sound. Though I don't think it would be a good solution.