Description
Proposal
Add a new lint NOOP_METHOD_CALL
, which fires on calls to methods which are known to do nothing. To start with, we would lint calls to the following methods:
<&T as Clone>::clone
<&T as Borrow>::borrow
<&T as Deref>::deref
<&T as ToOwned>::to_owned
These trait impls are useful in generic code (e.g. you pass an &T
to a function expecting a Clone
argument), but are pointless when called directly (e.g. &bool::clone(&true)
).
Note that we will intentionally not perform any kind of post-monomorphization checks. This lint will only fire on calls that are known to have the proper receiver (&T
) at the call site (where the user could just remove the call).
For example
struct Foo;
fn clone_it<T: Clone>(val: T) -> T {
val.clone() // No warning - we don't know if `T` is `&T`
}
fn main() {
let val = &Foo;
val.clone(); // WARNING: noop method call
clone_it(val);
}
This could be implemented via a new unstable #[noop]
attribute for most methods. <&T as ToOwned>::to_owned()
goes through a blanket impl, so we might need to hard-code it in some way. In the future, this could be stabilized and made available to user crates in some way.
Background
The immutable reference type &T
implements several traits (such as Clone
) with a no-op implementation that just returns the original reference. These trait impls are useful for working with generic code (you can pass an &T
to a function expecting a Clone
argument), but explicitly calling the method (e.g. &bool::clone(&true)
) is pointless.
Due to the way that method resolution works, calling (for example) string.clone()
on a reference may either invoke clone
on the pointee type (if the pointee type implements Clone
), or invoke clone
on the reference type itself. This can lead to confusing error messages (e.g. rust-lang/rust#78187), where a non-local change (adding/removing a trait impl) can make a normal-looking method call appear to return the wrong type.
Mentors or Reviewers
I'm planning to implement this if the MCP is accepted.
Process
The main points of the Major Change Process is as follows:
- File an issue describing the proposal.
- A compiler team member or contributor who is knowledgeable in the area can second by writing
@rustbot second
.- Finding a "second" suffices for internal changes. If however you are proposing a new public-facing feature, such as a
-C flag
, then full team check-off is required. - Compiler team members can initiate a check-off via
@rfcbot fcp merge
on either the MCP or the PR.
- Finding a "second" suffices for internal changes. If however you are proposing a new public-facing feature, such as a
- Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.
You can read more about Major Change Proposals on forge.
Comments
This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.