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

Expand float_cmp to structs with PartialEq #1685

Open
RazrFalcon opened this issue Apr 17, 2017 · 5 comments
Open

Expand float_cmp to structs with PartialEq #1685

RazrFalcon opened this issue Apr 17, 2017 · 5 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types

Comments

@RazrFalcon
Copy link

Example:

#[derive(PartialEq)]
struct Data {
    v: f64,
}

fn main() {
    let d1 = Data { v: 1.0 };
    let d2 = Data { v: 2.0 };

    // here should be a warning
    println!("{}", d1 == d2);
}
@clarfonthey
Copy link
Contributor

But this should only be a warning if the structure also implements PartialOrd.

@RazrFalcon
Copy link
Author

Doesn't PartialEq implemented without comparing the floats?

@clarfonthey
Copy link
Contributor

@RazrFalcon generally, the suggestion for floats is that instead of doing something like x == y, you'd do (x - y).abs() < z.

Although in this case, we have no idea if that's necessarily possible with these sorts of structs. You could warn on PartialEq for structs that are just floats, though, suggesting other alternatives.

@oli-obk oli-obk added L-correctness Lint: Belongs in the correctness lint group good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages T-middle Type: Probably requires verifiying types labels Apr 20, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Apr 26, 2017

I think this should not lint on the comparison site, but on the struct definition. Any struct deriving PartialEq and having float fields should be linted against.

@cschwan
Copy link

cschwan commented Sep 30, 2020

I think this should not lint on the comparison site, but on the struct definition. Any struct deriving PartialEq and having float fields should be linted against.

I think it's a bit more complicated than this, for instance consider comparing two Vec<f64>; in this case I would like the linter to complain at the comparison site, but I agree that when a struct has non-generic float fields it's best linted on the struct definition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy L-correctness Lint: Belongs in the correctness lint group T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

6 participants