Fix guidance of [float_cmp] and [float_cmp_const] to not incorrectly recommend f__::EPSILON as the error margin.
#13079
Conversation
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 rust-lang#6816
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @y21 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
There was a problem hiding this comment.
This is a really nice improvement, looks good to me. Just one question.
FWIW, there's an existing PR that also addresses the issue by removing the note about f__::EPSILON, but it also changes the lint's logic a fair bit in other ways (including removing the _const lint) so it seems good to land this "smaller" change on its own 👍
|
LGTM, thanks! @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Using
f32::EPSILONorf64::EPSILONas the floating-point equality comparison error margin is incorrect, yetfloat_cmphas 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 recommendf__::EPSILONas the error margin.changelog: [
float_cmp_const] Fix guidance to not incorrectly recommendf__::EPSILONas the error margin.Fixes #6816