Auto merge of rust-lang#13079 - sandersaares:u/sasaares/no-epsilon-gu…

…idance, r=y21

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

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 sample implementation for 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 equality comparison for cases where absolute error margins cannot be identified.

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

Fixes rust-lang#6816

clippy_lints/src/operators/float_cmp.rs

@@ -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`");
         });
     }
 }

clippy_lints/src/operators/mod.rs

@@ -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! {

tests/ui/float_cmp.rs

@@ -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;
@@ -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;

tests/ui/float_cmp.stderr

@@ -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
 

tests/ui/float_cmp_const.rs

@@ -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;
@@ -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`
 }

tests/ui/float_cmp_const.stderr

@@ -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