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

float_cmp: ignore when comparing with constant? #1142

Closed
birkenfeld opened this Issue Aug 5, 2016 · 4 comments

Comments

Projects
None yet
5 participants
@birkenfeld
Collaborator

birkenfeld commented Aug 5, 2016

Consider:

const ONE: f32 = 1.0;
fn main() {
    let v = 0.9;
    let _ = v == ONE;
}

These are probably cases where you want to check for the exact constant, because it is a common argument, a sentinel value, or an integer converted to floating-point due to external APIs, etc.

There is already a special case for 0 and +/-inf, but I'd add a case for this situation too.

@clippered

This comment has been minimized.

Contributor

clippered commented Oct 17, 2017

Hi, was just about to look at this one. Is this still the case at the moment? Correct me if I'm mistaken but it looks like it is already been covered by the lint.

warning: strict comparison of f32 or f64
--> input.rs:9:13
|
9 | let _ = v == ONE;
| ^^^^^^^^ help: consider comparing them within some error: (v - ONE).abs() < error
|
= note: #[warn(float_cmp)] on by default
note: std::f32::EPSILON and std::f64::EPSILON are available.
--> input.rs:9:13
|
9 | let _ = v == ONE;
| ^^^^^^^^
= help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.165/index.html#float_cmp

@oli-obk

This comment has been minimized.

Collaborator

oli-obk commented Nov 3, 2017

This has not been covered yet. I think what @birkenfeld suggests is to not lint if at least one side of the == is just the name of a constant.

@clippered

This comment has been minimized.

Contributor

clippered commented Nov 3, 2017

I see, thanks for the clarification @oli-obk . Do we also need to check if one side is an expression (e.g. (v+0.1) == ONE)? Or maybe it does not matter as long as one side is a constant.

@oli-obk

This comment has been minimized.

Collaborator

oli-obk commented Nov 3, 2017

As long as one side is a constant.

Instead of removing the linting if there are constants, we can add a restriction lint that still triggers. I'm fairly certain someone will want to forbid all float comparisons.

clippered added a commit to clippered/rust-clippy that referenced this issue Nov 4, 2017

oli-obk added a commit that referenced this issue Nov 20, 2017

Merge pull request #2203 from clippered/float_cmp_const
Fix #1142 float constant comparison lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment