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 in derive_hash_xor_eq with multiple Eq implementations #2025

Open
llogiq opened this issue Sep 6, 2017 · 5 comments
Open
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

@llogiq
Copy link
Contributor

llogiq commented Sep 6, 2017

In optional, I have both the standard derived Eq and Hash, plus one extra Eq impls on references for improved ergonomics. However, this additional Eq' impl triggers the derive_hash_xor_eq` lint. So perhaps the lint should look for existing derived implementations to avoid this false positive.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2017

Can you create a minimal repro that we can use as a test case? Preferably without extra crates.

@oli-obk oli-obk added the C-bug Category: Clippy is not doing the correct thing label Sep 6, 2017
@llogiq
Copy link
Contributor Author

llogiq commented Sep 6, 2017

https://play.rust-lang.org/?gist=18cdb2fd3b4b23e2d1d4d17c93537839&version=stable

use std::cmp::PartialEq;

#[derive(Debug, Eq, Hash, PartialEq)]
struct Foo;

impl<'a> PartialEq<Foo> for &'a Foo {
    fn eq(&self, _: &Foo) -> bool {
        true
    }
}

fn main() {
    assert_eq!(&Foo, Foo);
}

@vadixidav
Copy link

There also seems to be a false positive here in ndarray. It might be hard to track down why this is happening. I am just reporting it here.

@lo48576
Copy link
Contributor

lo48576 commented Sep 18, 2020

Now it seems that derive_ord_xor_partial_ord (#1621, #5848) has the same false positive.
It is enabled by default in the current beta (clippy 0.0.212 (755064b 2020-09-17)).

https://play.rust-lang.org/?version=beta&mode=debug&edition=2018&gist=12f17e27712b221f20555aab95a51869

    Checking playground v0.0.1 (/playground)
error: you are deriving `Ord` but have implemented `PartialOrd` explicitly
  --> src/lib.rs:4:37
   |
4  | #[derive(PartialEq, Eq, PartialOrd, Ord, Hash)]
   |                                     ^^^
   |
   = note: `#[deny(clippy::derive_ord_xor_partial_ord)]` on by default
note: `PartialOrd` implemented here
  --> src/lib.rs:14:1
   |
14 | / impl PartialOrd<Foo> for &Foo {
15 | |     fn partial_cmp(&self, o: &Foo) -> Option<std::cmp::Ordering> {
16 | |         // Redirect to the derived PartialOrd implementation.
17 | |         (**self).partial_cmp(o)
18 | |     }
19 | | }
   | |_^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

error: could not compile `playground`

To learn more, run the command again with --verbose.

@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 21, 2020
@Jarcho
Copy link
Contributor

Jarcho commented Jan 5, 2022

This has already been fixed.

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

6 participants