Skip to content

unconditional_recursion false positive in nightly #12245

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

Closed
zacknewman opened this issue Feb 8, 2024 · 2 comments
Closed

unconditional_recursion false positive in nightly #12245

zacknewman opened this issue Feb 8, 2024 · 2 comments
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

@zacknewman
Copy link

zacknewman commented Feb 8, 2024

Summary

Using nightly, cargo clippy incorrectly reports an unconditional_recursion warning.

Lint Name

unconditional_recursion

Reproducer

I tried this code:

#[derive(PartialEq)]
pub struct Foo;
impl PartialEq<&Self> for Foo {
    #[deny(unconditional_recursion)]
    fn eq(&self, other: &&Self) -> bool {
        // This is _not_ recursive since this calls the derived `PartialEq` implementation.
        *self == **other
    }
}
// Uncommenting below will lead to a compilation failure since the compiler correctly detects
// an actual recursion.
//impl PartialEq<Foo> for &Foo {
//    #[deny(unconditional_recursion)]
//    fn eq(&self, other: &Foo) -> bool {
//        *self == *other
//    }
//}
#[cfg(test)]
mod tests {
    use super::Foo;
    #[test]
    fn not_recursive() {
        let x = Foo;
        let y = &x;
        // If the call were recursive, this won't ever finish but it does.
        assert!(x == y);
        // Same as above, but more obviously illustrates it's relying on the
        // "recursive" implementation.
        assert!(<Foo as PartialEq<&Foo>>::eq(&x, &y));
    }
}

I saw this happen:

[zack@laptop src]$ cargo clippy
    Checking bar v0.1.0 (/home/zack/projects/bar)
warning: function cannot return without recursing
 --> src/lib.rs:5:5
  |
5 | /     fn eq(&self, other: &&Self) -> bool {
6 | |         // This is _not_ recursive since this calls the derived `PartialEq` implementation.
7 | |         *self == **other
8 | |     }
  | |_____^
  |
note: recursive call site
 --> src/lib.rs:7:9
  |
7 |         *self == **other
  |         ^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unconditional_recursion
  = note: `#[warn(clippy::unconditional_recursion)]` on by default

warning: `bar` (lib) generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
[zack@laptop src]$ cargo t
    Finished test [unoptimized + debuginfo] target(s) in 0.03s
     Running unittests src/lib.rs (/home/zack/projects/bar/target/debug/deps/bar-5f4c34b6090a377c)

running 1 test
test tests::not_recursive ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

   Doc-tests bar

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

I expected to see this happen:
no warnings

Version

[zack@laptop src]$ rustc -Vv
rustc 1.78.0-nightly (8ace7ea1f 2024-02-07)
binary: rustc
commit-hash: 8ace7ea1f7cbba7b4f031e66c54ca237a0d65de6
commit-date: 2024-02-07
host: x86_64-unknown-linux-gnu
release: 1.78.0-nightly
LLVM version: 17.0.6
[zack@laptop src]$ uname -a
Linux laptop 6.7.4-arch1-1 #1 SMP PREEMPT_DYNAMIC Mon, 05 Feb 2024 22:07:49 +0000 x86_64 GNU/Linux

Additional Labels

No response

@zacknewman zacknewman 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 8, 2024
@y21
Copy link
Member

y21 commented Feb 8, 2024

Thanks for the report! This was fixed in #12177. One of the test cases it adds looks pretty much identical to your case :)

// Not necessarily related to the issue but another FP from the http crate that was fixed with it:
// https://docs.rs/http/latest/src/http/header/name.rs.html#1424
// We used to simply peel refs from the LHS and RHS, so we couldn't differentiate
// between `PartialEq<T> for &T` and `PartialEq<&T> for T` impls.
#[derive(PartialEq)]
struct HeaderName;
impl<'a> PartialEq<&'a HeaderName> for HeaderName {
fn eq(&self, other: &&'a HeaderName) -> bool {
*self == **other
}
}

@zacknewman
Copy link
Author

Apologize for the noise. I clearly did a poor job at looking at recently closed issues.

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

No branches or pull requests

2 participants