RefCell<u32> -> Cell<u32>

[lengyijun]

What it does

Suggest using Cell<T> instead of RefCell<T>, T=u32,i32,f32 ...

Categories (optional)

clippy::perf

What is the advantage of the recommended code over the original code

• RefCell will calculate borrow at runtime, so RefCell has more overhead over Cell
• RefCell may panic at runtime

Drawbacks

Maybe some problems in generic type.

Example

struct S(RefCell<u32>);

Could be written as:

struct S(Cell<u32>);

[RalfJung]

I think this lint would be wrong -- the code might need the RefCell to create en &mut i32, which is not possible with a Cell.

[y21]

My thoughts:
I really like this idea.

At least for uses of RefCell that are local to a function (e.g. stored in a variable), this could be implemented by looking at (all) uses of it and checking if all calls to borrow(_mut) are immediately dereferenced or similar (think, *x.borrow_mut() += 1;, let y = *x.borrow();, ...), similar to how the useless_vec lint can sometimes suggest replacing a Vec with an array or a slice if you never call any Vec specific methods on it.
If we can prove that the reference is "unnecessary" in all instances, then we could safely suggest Cell.

Now for RefCell in fields, I think this could still be implemented in a similar way (store the variants of ADTs that contain a RefCell and for each use, check if the reference is unnecessary). However I vaguely remember a discussion about how "global analysis" is not something that clippy should do, so I'm not sure if this would get accepted. (There are some lints that keep track of item-specific state across multiple items and delay linting, though that's mostly just because of false positives. I'm not sure if that counts as global analysis, the term is a bit vague imo).
I suppose this might also not be ideal for libraries where the RefCell is directly exposed in its public API and we can't look at how this library is going to be used

(Also Cell even works with non-Copy types such as strings or vectors, but it requires swapping/replacing, which conveniently can still be done cheaply with .replace(Default::default()), although you need to remember to put it back in when you're done. Arguably this is just as likely to cause bugs as borrowing mutably twice with a RefCell, but instead of getting a panic you have the default empty string/vector. Not sure if this should also be considered if someone was to implement the lint.)

[GnomedDev]

This looks like a duplicate of #1