rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.3k stars 1.53k forks source link

Lint against Arc<impl Copy> #13237

Open Kyuuhachi opened 1 month ago

Kyuuhachi commented 1 month ago

What it does

There is very little reason to wrap a Copy type in Rc/Arc: just copying the value directly is usually faster and certainly more ergonomic. Additionally, it's a decently common beginner mistake to try to do Arc::new(&value), which of course doesn't work as wanted. This lint would catch that as well as a side effect.

I don't know much about designing lints, but I think what should be linted against is calls of Arc::new with a Copy parameter, and also explicit mentions of Arc<T> inside structs and variables and the like. The latter ought to catch most instances where Arcs are created via for example Into or Default.

It might also be good to warn against Arc<&mut T> even though it is not Copy, because it's useless.

Of course, everything said here also applies to Rc.

Advantage

Drawbacks

There may be rare false positives where an Arc is actually desirable. One such case is large arrays: here we could either apply pass-by-value-size-limit or some similar parameter, or just leave people to #[allow] it in those rare cases. Another case is Arc<()> where it's used purely for the count, but this is vanishingly rare and unidiomatic.

Example

#[derive(Clone, Default)]
struct Wrapper {
    value: Arc<u64>
}

let thing = Arc::new(&thing);

Could be written as:

#[derive(Clone, Default)]
struct Wrapper {
    value: u64
}

let thing = Arc::new(thing);
lukaslueg commented 1 month ago

Similar to #1 (!)