redundant_clone in Rc context ?

[o0Ignition0o]

We had a small discussion on it here: mozilla/neqo#374 (comment).
I wonder if we could tell a user to use Rc::clone(&foo) if the linter warns about a redundant clone in Rc context ?

If so I would love to work on it, but that would be my first PR to clippy, so it would probably require someone to mentor me.

Thanks for your time :)

[flip1995]

The lint is implemented in redundant_clone.rs, so somewhere in there Rc could be special cased. Probably somewhere inside this if:

rust-clippy/clippy_lints/src/redundant_clone.rs

Line 211 in 05b4603

if !used_later {

Since @sinkuu completely rewrote this lint, maybe he can give a little bit more insight.

---

For how to write tests in Clippy, you can read this part of the documentation: https://github.com/rust-lang/rust-clippy/blob/master/doc/adding_lints.md#testing

[sinkuu]

The type of the value being .clone()d is available here as arg_ty, so you could add something like if is_rc(arg_ty) { continue; } below that.

rust-clippy/clippy_lints/src/redundant_clone.rs

Line 115 in 05b4603

let (fn_def_id, arg, arg_ty, _) = unwrap_or_continue!(is_call_with_ref_arg(cx, mir, &terminator.kind));

I think Rc::clone suggestion is orthogonal to this lint and should be a new lint.

[o0Ignition0o]

The type of the value being .clone()d is available here as arg_ty, so you could add something like if is_rc(arg_ty) { continue; } below that.

Pretty cool I'll try to work on it tomorrow or next week :D

I think Rc::clone suggestion is orthogonal to this lint and should be a new lint.

Would it make sense to create a redundant_rc_clone.rs file ?
maybe that ought to be part of an other commit / PR ?

[sinkuu]

redundant_rc_clone

Do you mean to suggest Rc::clone only when it is actually redundant? Then it may be fine to include the feature in redundant_clone.

(I don't see the point in doing so though. I was thinking of a lint that replaces all rc.clone() with Rc::clone(&rc)...)

[o0Ignition0o]

Do you mean to suggest Rc::clone only when it is actually redundant? Then it may be fine to include the feature in redundant_clone.

That's what I initially thought of but the lint you mention (replacing rc.clone() with Rc::clone(&rc)) would be a great improvement IMO, it would make things explicit, and still allow redundant_clone to do its job in RC context.

I'm however not aware of the actual process to discuss a new lint and create a consensus among the clippy contributors ^^'.

[flip1995]

I'm however not aware of the actual process to discuss a new lint and create a consensus among the clippy contributors

You can do 2 things:

1. Open an issue for a (style) lint rc.clone() -> Rc::clone(&rc) and wait for someone to take this issue or
2. Just implement it yourself and refer to this issue.

If you decide to do (2.) and need any help with that, just ask here or open a WIP PR :)

About the consensus: If people disagree with the lint, we can still move it to a allow-by-default group.

[o0Ignition0o]

I would love to try to implement it myself !

I have a couple of things to finish before I tackle it though, but feel free to assign me to it :)

[lengyijun]

clone_on_ref_ptr lints on it.