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

Following cmp_owned hint leads to different behaviour #7769

Closed
Schaeff opened this issue Oct 5, 2021 · 2 comments
Closed

Following cmp_owned hint leads to different behaviour #7769

Schaeff opened this issue Oct 5, 2021 · 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

@Schaeff
Copy link

Schaeff commented Oct 5, 2021

I have code which uses different PartialEq implementations, which is arguably not the best design, but clippy complains about unnecessary allocations and its suggestion ends up using another PartialEq implementation.

Lint name: cmp_owned

I tried this code, before the suggestion it prints Strict! which is the derived PartialEq, after it prints Optimistic! which is the more optimistic PartialEq.

#[derive(PartialEq)]
enum MaybeU32 {
    Unknown,
    Known(u32),
}

impl From<u32> for MaybeU32 {
    fn from(a: u32) -> Self {
        MaybeU32::Known(a)
    }
}

impl PartialEq<u32> for MaybeU32 {
    fn eq(&self, other: &u32) -> bool {
        use MaybeU32::*;
        match self {
            Known(number) => number == other,
            _ => true, // we optimistically return true now because it may end up being the case
        }
    }
}

fn main() {
    if MaybeU32::Unknown == MaybeU32::from(32u32) {
        println!("Optimistic!");
    } else {
        println!("Strict!");
    }
}

Meta

Rust version (rustc -Vv):

rustc 1.46.0-nightly (f455e46ea 2020-06-20)
binary: rustc
commit-hash: f455e46eae1a227d735091091144601b467e1565
commit-date: 2020-06-20
host: x86_64-unknown-linux-gnu
release: 1.46.0-nightly
LLVM version: 10.0
@Schaeff Schaeff 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 Oct 5, 2021
@camsteffen
Copy link
Contributor

Your PartialEq implementation does not satisfy the transitive equality requirement.

@flip1995
Copy link
Member

flip1995 commented Oct 8, 2021

PartialEq has the documented contract:

The equality relation == must satisfy the following conditions (for all a, b, c of type A, B, C):

  • Symmetric: if A: PartialEq<B> and B: PartialEq<A>, then a == b implies b == a; and
  • Transitive: if A: PartialEq<B> and B: PartialEq<C> and A: PartialEq<C>, then a == b and b == c implies a == c.

Since your implementation violates this, Clippy can't make a sensible suggestion. This is because Clippy assumes that you're implementation meets all preconditions.

This is therefore not a bug in Clippy.

@flip1995 flip1995 closed this as completed Oct 8, 2021
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

3 participants