Skip to content

Fix guidance of [float_cmp] and [float_cmp_const] to not incorrectly recommend f__::EPSILON as the error margin. #13079

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

Merged
merged 3 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion clippy_lints/src/operators/float_cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ pub(crate) fn check<'tcx>(
Applicability::HasPlaceholders, // snippet
);
}
diag.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`");
});
}
}
Expand Down
122 changes: 88 additions & 34 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,69 +574,123 @@ declare_clippy_lint! {
/// implement equality for a type involving floats).
///
/// ### Why is this bad?
/// Floating point calculations are usually imprecise, so
/// asking if two values are *exactly* equal is asking for trouble. For a good
/// guide on what to do, see [the floating point
/// guide](http://www.floating-point-gui.de/errors/comparison).
/// Floating point calculations are usually imprecise, so asking if two values are *exactly*
/// equal is asking for trouble because arriving at the same logical result via different
/// routes (e.g. calculation versus constant) may yield different values.
///
/// ### Example
///
/// ```no_run
/// let x = 1.2331f64;
/// let y = 1.2332f64;
/// let a: f64 = 1000.1;
/// let b: f64 = 0.2;
/// let x = a + b;
/// let y = 1000.3; // Expected value.
///
/// // Actual value: 1000.3000000000001
/// println!("{x}");
///
/// if y == 1.23f64 { }
/// if y != x {} // where both are floats
/// let are_equal = x == y;
/// println!("{are_equal}"); // false
/// ```
///
/// Use instead:
/// The correct way to compare floating point numbers is to define an allowed error margin. This
/// may be challenging if there is no "natural" error margin to permit. Broadly speaking, there
/// are two cases:
///
/// 1. If your values are in a known range and you can define a threshold for "close enough to
/// be equal", it may be appropriate to define an absolute error margin. For example, if your
/// data is "length of vehicle in centimeters", you may consider 0.1 cm to be "close enough".
/// 1. If your code is more general and you do not know the range of values, you should use a
/// relative error margin, accepting e.g. 0.1% of error regardless of specific values.
///
/// For the scenario where you can define a meaningful absolute error margin, consider using:
///
/// ```no_run
/// # let x = 1.2331f64;
/// # let y = 1.2332f64;
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
/// // let error_margin = std::f64::EPSILON;
/// if (y - 1.23f64).abs() < error_margin { }
/// if (y - x).abs() > error_margin { }
/// let a: f64 = 1000.1;
/// let b: f64 = 0.2;
/// let x = a + b;
/// let y = 1000.3; // Expected value.
///
/// const ALLOWED_ERROR_VEHICLE_LENGTH_CM: f64 = 0.1;
/// let within_tolerance = (x - y).abs() < ALLOWED_ERROR_VEHICLE_LENGTH_CM;
/// println!("{within_tolerance}"); // true
/// ```
///
/// NB! Do not use `f64::EPSILON` - while the error margin is often called "epsilon", this is
/// a different use of the term that is not suitable for floating point equality comparison.
/// Indeed, for the example above using `f64::EPSILON` as the allowed error would return `false`.
///
/// For the scenario where no meaningful absolute error can be defined, refer to
/// [the floating point guide](https://www.floating-point-gui.de/errors/comparison)
/// for a reference implementation of relative error based comparison of floating point values.
/// `MIN_NORMAL` in the reference implementation is equivalent to `MIN_POSITIVE` in Rust.
#[clippy::version = "pre 1.29.0"]
pub FLOAT_CMP,
pedantic,
"using `==` or `!=` on float values instead of comparing difference with an epsilon"
"using `==` or `!=` on float values instead of comparing difference with an allowed error"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for (in-)equality comparisons on floating-point
/// value and constant, except in functions called `*eq*` (which probably
/// Checks for (in-)equality comparisons on constant floating-point
/// values (apart from zero), except in functions called `*eq*` (which probably
/// implement equality for a type involving floats).
///
/// ### Why restrict this?
/// Floating point calculations are usually imprecise, so
/// asking if two values are *exactly* equal is asking for trouble. For a good
/// guide on what to do, see [the floating point
/// guide](http://www.floating-point-gui.de/errors/comparison).
/// Floating point calculations are usually imprecise, so asking if two values are *exactly*
/// equal is asking for trouble because arriving at the same logical result via different
/// routes (e.g. calculation versus constant) may yield different values.
///
/// ### Example
///
/// ```no_run
/// let x: f64 = 1.0;
/// const ONE: f64 = 1.00;
/// let a: f64 = 1000.1;
/// let b: f64 = 0.2;
/// let x = a + b;
/// const Y: f64 = 1000.3; // Expected value.
///
/// // Actual value: 1000.3000000000001
/// println!("{x}");
///
/// if x == ONE { } // where both are floats
/// let are_equal = x == Y;
/// println!("{are_equal}"); // false
/// ```
///
/// Use instead:
/// The correct way to compare floating point numbers is to define an allowed error margin. This
/// may be challenging if there is no "natural" error margin to permit. Broadly speaking, there
/// are two cases:
///
/// 1. If your values are in a known range and you can define a threshold for "close enough to
/// be equal", it may be appropriate to define an absolute error margin. For example, if your
/// data is "length of vehicle in centimeters", you may consider 0.1 cm to be "close enough".
/// 1. If your code is more general and you do not know the range of values, you should use a
/// relative error margin, accepting e.g. 0.1% of error regardless of specific values.
///
/// For the scenario where you can define a meaningful absolute error margin, consider using:
///
/// ```no_run
/// # let x: f64 = 1.0;
/// # const ONE: f64 = 1.00;
/// let error_margin = f64::EPSILON; // Use an epsilon for comparison
/// // Or, if Rust <= 1.42, use `std::f64::EPSILON` constant instead.
/// // let error_margin = std::f64::EPSILON;
/// if (x - ONE).abs() < error_margin { }
/// let a: f64 = 1000.1;
/// let b: f64 = 0.2;
/// let x = a + b;
/// const Y: f64 = 1000.3; // Expected value.
///
/// const ALLOWED_ERROR_VEHICLE_LENGTH_CM: f64 = 0.1;
/// let within_tolerance = (x - Y).abs() < ALLOWED_ERROR_VEHICLE_LENGTH_CM;
/// println!("{within_tolerance}"); // true
/// ```
///
/// NB! Do not use `f64::EPSILON` - while the error margin is often called "epsilon", this is
/// a different use of the term that is not suitable for floating point equality comparison.
/// Indeed, for the example above using `f64::EPSILON` as the allowed error would return `false`.
///
/// For the scenario where no meaningful absolute error can be defined, refer to
/// [the floating point guide](https://www.floating-point-gui.de/errors/comparison)
/// for a reference implementation of relative error based comparison of floating point values.
/// `MIN_NORMAL` in the reference implementation is equivalent to `MIN_POSITIVE` in Rust.
#[clippy::version = "pre 1.29.0"]
pub FLOAT_CMP_CONST,
restriction,
"using `==` or `!=` on float constants instead of comparing difference with an epsilon"
"using `==` or `!=` on float constants instead of comparing difference with an allowed error"
}

declare_clippy_lint! {
Expand Down
6 changes: 0 additions & 6 deletions tests/ui/float_cmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,16 @@ fn main() {
twice(ONE) != ONE;
ONE as f64 != 2.0;
//~^ ERROR: strict comparison of `f32` or `f64`
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
ONE as f64 != 0.0; // no error, comparison with zero is ok

let x: f64 = 1.0;

x == 1.0;
//~^ ERROR: strict comparison of `f32` or `f64`
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
x != 0f64; // no error, comparison with zero is ok

twice(x) != twice(ONE as f64);
//~^ ERROR: strict comparison of `f32` or `f64`
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

x < 0.0; // no errors, lower or greater comparisons need no fuzzyness
x > 0.0;
Expand All @@ -105,17 +102,14 @@ fn main() {
ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; // ok, because lhs is zero regardless of i
NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j];
//~^ ERROR: strict comparison of `f32` or `f64`
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

let a1: [f32; 1] = [0.0];
let a2: [f32; 1] = [1.1];

a1 == a2;
//~^ ERROR: strict comparison of `f32` or `f64` arrays
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
a1[0] == a2[0];
//~^ ERROR: strict comparison of `f32` or `f64`
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

// no errors - comparing signums is ok
let x32 = 3.21f32;
Expand Down
21 changes: 5 additions & 16 deletions tests/ui/float_cmp.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,49 +4,38 @@ error: strict comparison of `f32` or `f64`
LL | ONE as f64 != 2.0;
| ^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(ONE as f64 - 2.0).abs() > error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
= note: `-D clippy::float-cmp` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::float_cmp)]`

error: strict comparison of `f32` or `f64`
--> tests/ui/float_cmp.rs:79:5
--> tests/ui/float_cmp.rs:78:5
|
LL | x == 1.0;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(x - 1.0).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui/float_cmp.rs:84:5
--> tests/ui/float_cmp.rs:82:5
|
LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(twice(x) - twice(ONE as f64)).abs() > error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui/float_cmp.rs:106:5
--> tests/ui/float_cmp.rs:103:5
|
LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(NON_ZERO_ARRAY[i] - NON_ZERO_ARRAY[j]).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` arrays
--> tests/ui/float_cmp.rs:113:5
--> tests/ui/float_cmp.rs:109:5
|
LL | a1 == a2;
| ^^^^^^^^
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64`
--> tests/ui/float_cmp.rs:116:5
--> tests/ui/float_cmp.rs:111:5
|
LL | a1[0] == a2[0];
| ^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(a1[0] - a2[0]).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: aborting due to 6 previous errors

8 changes: 0 additions & 8 deletions tests/ui/float_cmp_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,21 @@ fn main() {
// has errors
1f32 == ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
TWO == ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
TWO != ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
ONE + ONE == TWO;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
let x = 1;
x as f32 == ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

let v = 0.9;
v == ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
v != ONE;
//~^ ERROR: strict comparison of `f32` or `f64` constant
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

// no errors, lower than or greater than comparisons
v < ONE;
Expand Down Expand Up @@ -70,5 +63,4 @@ fn main() {
// has errors
NON_ZERO_ARRAY == NON_ZERO_ARRAY2;
//~^ ERROR: strict comparison of `f32` or `f64` constant arrays
//~| NOTE: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
}
29 changes: 7 additions & 22 deletions tests/ui/float_cmp_const.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,65 +4,50 @@ error: strict comparison of `f32` or `f64` constant
LL | 1f32 == ONE;
| ^^^^^^^^^^^ help: consider comparing them within some margin of error: `(1f32 - ONE).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`
= note: `-D clippy::float-cmp-const` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::float_cmp_const)]`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:19:5
--> tests/ui/float_cmp_const.rs:18:5
|
LL | TWO == ONE;
| ^^^^^^^^^^ help: consider comparing them within some margin of error: `(TWO - ONE).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:22:5
--> tests/ui/float_cmp_const.rs:20:5
|
LL | TWO != ONE;
| ^^^^^^^^^^ help: consider comparing them within some margin of error: `(TWO - ONE).abs() > error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:25:5
--> tests/ui/float_cmp_const.rs:22:5
|
LL | ONE + ONE == TWO;
| ^^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(ONE + ONE - TWO).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:29:5
--> tests/ui/float_cmp_const.rs:25:5
|
LL | x as f32 == ONE;
| ^^^^^^^^^^^^^^^ help: consider comparing them within some margin of error: `(x as f32 - ONE).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:34:5
--> tests/ui/float_cmp_const.rs:29:5
|
LL | v == ONE;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(v - ONE).abs() < error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant
--> tests/ui/float_cmp_const.rs:37:5
--> tests/ui/float_cmp_const.rs:31:5
|
LL | v != ONE;
| ^^^^^^^^ help: consider comparing them within some margin of error: `(v - ONE).abs() > error_margin`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: strict comparison of `f32` or `f64` constant arrays
--> tests/ui/float_cmp_const.rs:71:5
--> tests/ui/float_cmp_const.rs:64:5
|
LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error_margin`

error: aborting due to 8 previous errors