diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b53b34b3b2b..6ceb6176bd7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -367,6 +367,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) { arithmetic::INTEGER_ARITHMETIC, array_indexing::INDEXING_SLICING, assign_ops::ASSIGN_OPS, + misc::FLOAT_CMP_CONST, ]); reg.register_lint_group("clippy_pedantic", vec![ diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index e1d350a9ad2..c9c404c1740 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -13,6 +13,7 @@ use utils::{get_item_name, get_parent_expr, implements_trait, in_constant, in_ma span_lint_and_then, walk_ptrs_ty}; use utils::sugg::Sugg; use syntax::ast::{FloatTy, LitKind, CRATE_NODE_ID}; +use consts::constant; /// **What it does:** Checks for function arguments and let bindings denoted as /// `ref`. @@ -200,6 +201,27 @@ declare_lint! { "using 0 as *{const, mut} T" } +/// **What it does:** Checks for (in-)equality comparisons on floating-point +/// value and constant, except in functions called `*eq*` (which probably +/// 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). +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// const ONE == 1.00f64 +/// x == ONE // where both are floats +/// ``` +declare_restriction_lint! { + pub FLOAT_CMP_CONST, + "using `==` or `!=` on float constants instead of comparing difference with an epsilon" +} + #[derive(Copy, Clone)] pub struct Pass; @@ -214,7 +236,8 @@ impl LintPass for Pass { REDUNDANT_PATTERN, USED_UNDERSCORE_BINDING, SHORT_CIRCUIT_STATEMENT, - ZERO_PTR + ZERO_PTR, + FLOAT_CMP_CONST ) } } @@ -334,7 +357,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { return; } } - span_lint_and_then(cx, FLOAT_CMP, expr.span, "strict comparison of f32 or f64", |db| { + let (lint, msg) = if is_named_constant(cx, left) || is_named_constant(cx, right) { + (FLOAT_CMP_CONST, "strict comparison of f32 or f64 constant") + } else { + (FLOAT_CMP, "strict comparison of f32 or f64") + }; + span_lint_and_then(cx, lint, expr.span, msg, |db| { let lhs = Sugg::hir(cx, left, ".."); let rhs = Sugg::hir(cx, right, ".."); @@ -423,6 +451,14 @@ fn check_nan(cx: &LateContext, path: &Path, expr: &Expr) { } } +fn is_named_constant<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { + if let Some((_, res)) = constant(cx, expr) { + res + } else { + false + } +} + fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { let parent_item = cx.tcx.hir.get_parent(expr.id); let parent_def_id = cx.tcx.hir.local_def_id(parent_item); diff --git a/tests/ui/float_cmp_const.rs b/tests/ui/float_cmp_const.rs new file mode 100644 index 00000000000..adf2ab70368 --- /dev/null +++ b/tests/ui/float_cmp_const.rs @@ -0,0 +1,45 @@ + + + +#![warn(float_cmp_const)] +#![allow(float_cmp)] +#![allow(unused, no_effect, unnecessary_operation)] + +const ONE: f32 = 1.0; +const TWO: f32 = 2.0; + +fn eq_one(x: f32) -> bool { + if x.is_nan() { false } else { x == ONE } // no error, inside "eq" fn +} + +fn main() { + // has errors + 1f32 == ONE; + TWO == ONE; + TWO != ONE; + ONE + ONE == TWO; + 1 as f32 == ONE; + + let v = 0.9; + v == ONE; + v != ONE; + + // no errors, lower than or greater than comparisons + v < ONE; + v > ONE; + v <= ONE; + v >= ONE; + + // no errors, zero and infinity values + ONE != 0f32; + TWO == 0f32; + ONE != ::std::f32::INFINITY; + ONE == ::std::f32::NEG_INFINITY; + + // no errors, but will warn float_cmp if '#![allow(float_cmp)]' above is removed + let w = 1.1; + v == w; + v != w; + v == 1.0; + v != 1.0; +} diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr new file mode 100644 index 00000000000..fe277de28dd --- /dev/null +++ b/tests/ui/float_cmp_const.stderr @@ -0,0 +1,85 @@ +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:17:5 + | +17 | 1f32 == ONE; + | ^^^^^^^^^^^ help: consider comparing them within some error: `(1f32 - ONE).abs() < error` + | + = note: `-D float-cmp-const` implied by `-D warnings` +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:17:5 + | +17 | 1f32 == ONE; + | ^^^^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:18:5 + | +18 | TWO == ONE; + | ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:18:5 + | +18 | TWO == ONE; + | ^^^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:19:5 + | +19 | TWO != ONE; + | ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:19:5 + | +19 | TWO != ONE; + | ^^^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:20:5 + | +20 | ONE + ONE == TWO; + | ^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE + ONE - TWO).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:20:5 + | +20 | ONE + ONE == TWO; + | ^^^^^^^^^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:21:5 + | +21 | 1 as f32 == ONE; + | ^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(1 as f32 - ONE).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:21:5 + | +21 | 1 as f32 == ONE; + | ^^^^^^^^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:24:5 + | +24 | v == ONE; + | ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:24:5 + | +24 | v == ONE; + | ^^^^^^^^ + +error: strict comparison of f32 or f64 constant + --> $DIR/float_cmp_const.rs:25:5 + | +25 | v != ONE; + | ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error` + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp_const.rs:25:5 + | +25 | v != ONE; + | ^^^^^^^^ +