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

False positive for derive_partial_eq_without_eq when using PhantomData #8867

Closed
Herschel opened this issue May 22, 2022 · 7 comments · Fixed by #8869
Closed

False positive for derive_partial_eq_without_eq when using PhantomData #8867

Herschel opened this issue May 22, 2022 · 7 comments · Fixed by #8869
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

@Herschel
Copy link
Contributor

Summary

A struct with PhantomData will trigger the derive_partial_eq_without_eq even though it derives both Eq, PartialEq.

clippy 0.1.63 (9257f5a 2022-05-21)

Lint Name

derive_partial_eq_without_eq

Reproducer

I tried this code:

#[derive(Eq, PartialEq)]
struct Index<T>(std::marker::PhantomData<T>);

I saw this happen:

warning: you are deriving `PartialEq` and can implement `Eq`
 --> src\main.rs:1:14
  |
1 | #[derive(Eq, PartialEq)]
  |              ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
  |
  = note: `#[warn(clippy::derive_partial_eq_without_eq)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq

I expected to see this happen:
No warning.

Version

rustc 1.63.0-nightly (9257f5aad 2022-05-21)
binary: rustc
commit-hash: 9257f5aad02b65665a6e23e5b92938548302e129
commit-date: 2022-05-21
host: x86_64-pc-windows-msvc
release: 1.63.0-nightly
LLVM version: 14.0.4

Additional Labels

No response

@Herschel Herschel 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 May 22, 2022
@Herschel
Copy link
Contributor Author

@izik1
Copy link
Contributor

izik1 commented May 22, 2022

I was about to post an almost identical issue, so instead I'll just affirm this happens, and that I found a slightly smaller repro (dependencies wise, not bytes wise):

#[derive(Eq, PartialEq)]
pub struct Outer<T>(Inner<T>);

struct Inner<T>(T);

impl<T> PartialEq for Inner<T> {
    fn eq(&self, other: &Self) -> bool {
        loop {}
    }
}

impl<T> Eq for Inner<T> {}

(note that the loop there causes a different warning- about empty loops and CPU cycles, completely true in this context- which could be removed by using panic! instead, but there's no clear guideline on which to choose in a situation like this so I went with my gut)

Note that the very similar:

#[derive(Eq, PartialEq)]
pub struct Outer<T>(Inner<T>);

struct Inner<T>(T);

impl<T> PartialEq for Inner<T> where T: PartialEq {
    fn eq(&self, other: &Self) -> bool {
        loop {}
    }
}

impl<T> Eq for Inner<T> where T: Eq {}

does not trigger this, so the issue seems to be with the more general bounds on Inner vs Outer

(immediately edited to mark the last snippet as rust)

@izik1
Copy link
Contributor

izik1 commented May 22, 2022

Note that this is a very very fresh regression:
last "good" nightly (using git bisect terms):

rustc -Vv
rustc 1.63.0-nightly (e6a4afc3a 2022-05-20)
binary: rustc
commit-hash: e6a4afc3af2d2a53f91fc8a77bdfe94bea375b29
commit-date: 2022-05-20
host: x86_64-unknown-linux-gnu
release: 1.63.0-nightly
LLVM version: 14.0.4

Which means that the first "bad" nightly is:

rustc 1.63.0-nightly (9257f5aad 2022-05-21)
binary: rustc
commit-hash: 9257f5aad02b65665a6e23e5b92938548302e129
commit-date: 2022-05-21
host: x86_64-unknown-linux-gnu
release: 1.63.0-nightly
LLVM version: 14.0.4

@Bromeon
Copy link

Bromeon commented May 22, 2022

Can confirm this, just hit the issue in a CI run.

I was first thinking Clippy had a bug with ordering #[derive(Eq, PartialEq)] as it suggests

help: consider deriving `Eq` as well: `PartialEq, Eq`.

However, changing to #[derive(PartialEq, Eq)] still caused the same issue.

bors bot pushed a commit to jonasbb/serde_with that referenced this issue May 23, 2022
bors bot added a commit to jonasbb/serde_with that referenced this issue May 23, 2022
457: Fix clippy false positive r=jonasbb a=jonasbb

rust-lang/rust-clippy#8867

bors merge

Co-authored-by: Jonas Bushart <jonas@bushart.org>
bors bot pushed a commit to jonasbb/serde_with that referenced this issue May 23, 2022
bors bot added a commit to jonasbb/serde_with that referenced this issue May 23, 2022
457: Fix clippy false positive r=jonasbb a=jonasbb

rust-lang/rust-clippy#8867

bors merge

Co-authored-by: Jonas Bushart <jonas@bushart.org>
@boydjohnson
Copy link

boydjohnson commented May 24, 2022

This bug got me recently, and I wanted to point out a small change can be made to @izik1 2nd example and make it trip up the bug.

#[derive(Debug, PartialEq, Eq)]
pub struct Foo<T: Debug> {
    bar: Bar<T>,
}

#[derive(Debug)]
pub enum Bar<T> {
    One(T),
    Two,
}

impl<T> PartialEq for Bar<T>
where
    T: PartialEq + Debug,
{
    fn eq(&self, other: &Bar<T>) -> bool {
        match (self, other) {
            (Self::One(_), Self::Two) => false,
            (Self::One(inner), Self::One(other_inner)) => inner.eq(other_inner),
            (Self::Two, Self::Two) => true,
            (Self::Two, Self::One(_)) => false,
        }
    }
}

impl<T: Debug + PartialEq> Eq for Bar<T> {}

Adding the second bound, and having it on the type parameter in the struct, will cause the clippy error.

warning: you are deriving `PartialEq` and can implement `Eq`
 --> src/main.rs:5:17
  |
5 | #[derive(Debug, PartialEq, Eq)]
  |                 ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
  |

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=0887f13a6f08bd514ab4070bdb837b8e

@WorldSEnder
Copy link

@bstrie
Copy link
Contributor

bstrie commented Jun 1, 2022

Same here, I suggest that this lint should be removed from clippy::all while this bug exists, as it's causing reams of false positives on my end.

@bors bors closed this as completed in 7572b6b Jun 1, 2022
relrelb added a commit to relrelb/ruffle that referenced this issue Aug 16, 2022
rust-lang/rust-clippy#8867 is fixed, and
a false positive is no longer reported.
Herschel pushed a commit to ruffle-rs/ruffle that referenced this issue Aug 16, 2022
dmoka added a commit to galacticcouncil/Basilisk-node that referenced this issue Sep 9, 2022
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.

6 participants