Skip to content

Commit

Permalink
Replace incorrect suggested fix for float_cmp
Browse files Browse the repository at this point in the history
Using `f32::EPSILON` or `f64::EPSILON` as the floating-point equality comparison error margin is incorrect, yet `float_cmp` has until now recommended this be done. This change fixes the given guidance (both in docs and compiler hints) to not reference these unsuitable constants.

Instead, the guidance now clarifies that the scenarios in which an absolute error margin is usable, provides a reference implementation of using a user-defined absolute error margin (as an absolute error margin can only be used-defined and may be different for different comparisons) and references the floating point guide for a reference implementation of relative error based equaltiy comparison for when absolute error margin cannot be used.

changelog: Fix guidance of [`float_cmp`] and [`float_cmp_const`] to not incorrectly recommend `f64::EPSILON` as the error margin.

Fixes #6816
  • Loading branch information
sandersaares committed Jul 9, 2024
1 parent 1de41b1 commit a067cd2
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 88 deletions.
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
114 changes: 79 additions & 35 deletions clippy_lints/src/operators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -574,69 +574,113 @@ 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 { }
/// 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).
/// ### Why is this bad?
/// 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.
///
/// if x == ONE { } // where both are floats
/// // Actual value: 1000.3000000000001
/// println!("{x}");
///
/// 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 { }
/// 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

0 comments on commit a067cd2

Please sign in to comment.