Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect: this expression borrows a value the compiler would automatically borrow #8407

Closed
ritchie46 opened this issue Feb 10, 2022 · 1 comment · Fixed by #8441
Closed
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@ritchie46
Copy link

Summary

Clippy claims that a compiler would automatically borrow specific user borrows. E.g. (&impl Struct). These explicit borrows are done to dispatch to a implementation on the reference. So that owned code can utilize the same implementation.

Lint Name

needless_borrow

Reproducer

I tried this code:

use std::ops::Add;

struct Bar {
    val: i32
}

impl Add for &Bar {
    type Output = Bar;

    fn add(self, rhs: Self) -> Self::Output {
        let new = self.val + rhs.val;
        Bar {
            val: new
        }
    }
}

impl Add for Bar {
    type Output = Bar;

    fn add(self, rhs: Self) -> Self::Output {
        // dispatch to ref impl
        (&self).add(&rhs)
    }
}

I saw this happen:

warning: this expression borrows a value the compiler would automatically borrow
  --> src/main.rs:58:9
   |
58 |         (&self).add(&rhs)
   |         ^^^^^^^ help: change this to: `self`
   |
   = note: `#[warn(clippy::needless_borrow)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow


I expected to see this happen:

No warning.

Doing as the lint suggests, leads to recursive call site

warning: function cannot return without recursing
  --> src/main.rs:56:5
   |
56 |     fn add(self, rhs: Self) -> Self::Output {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
57 |         // dispatch to ref impl
58 |         self.add(rhs)
   |         ------------- recursive call site
   |
   = note: `#[warn(unconditional_recursion)]` on by default
   = help: a `loop` may express intention better if this is on purpose

So that's a catch 22.

Version

rustc 1.60.0-nightly (0c292c966 2022-02-08)
binary: rustc
commit-hash: 0c292c9667f1b202a9150d58bdd2e89e3e803996
commit-date: 2022-02-08
host: x86_64-unknown-linux-gnu
release: 1.60.0-nightly
LLVM version: 13.0.0

Additional Labels

No response

@ritchie46 ritchie46 added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Feb 10, 2022
@georgmu
Copy link

georgmu commented Feb 10, 2022

I have the same warning on a slightly different setup, which can be reduced to the following piece of code:

struct A (u32);
struct B (u32);

impl<'a> From<&'a A> for B {
    fn from(a: &A) -> Self {
        Self(a.0)
    }
}

fn main() {
    let a = A(1);
    let b: B = (&a).into(); // <- wrong warning here
    println!("b: {}", b.0);
}

No clippy warning with stable, but with nightly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants