Skip to content

Add a NOOP_METHOD_CALL lint for methods which should never be directly called #375

Closed
@Aaron1011

Description

@Aaron1011

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.
  • 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-compilerAdd this label so rfcbot knows to poll the compiler teammajor-changeA proposal to make a major change to rustcmajor-change-acceptedA major change proposal that was accepted

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions