Fix #1142 float constant comparison lint

This commit is contained in:
clippered 2017-11-04 19:32:58 +11:00
parent 0c43b60dd4
commit 2787a60fc2
4 changed files with 155 additions and 2 deletions

View file

@ -357,6 +357,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
arithmetic::INTEGER_ARITHMETIC, arithmetic::INTEGER_ARITHMETIC,
array_indexing::INDEXING_SLICING, array_indexing::INDEXING_SLICING,
assign_ops::ASSIGN_OPS, assign_ops::ASSIGN_OPS,
misc::FLOAT_CMP_CONST,
]); ]);
reg.register_lint_group("clippy_pedantic", vec![ reg.register_lint_group("clippy_pedantic", vec![

View file

@ -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}; span_lint_and_then, walk_ptrs_ty};
use utils::sugg::Sugg; use utils::sugg::Sugg;
use syntax::ast::{FloatTy, LitKind, CRATE_NODE_ID}; use syntax::ast::{FloatTy, LitKind, CRATE_NODE_ID};
use consts::constant;
/// **What it does:** Checks for function arguments and let bindings denoted as /// **What it does:** Checks for function arguments and let bindings denoted as
/// `ref`. /// `ref`.
@ -200,6 +201,27 @@ declare_lint! {
"using 0 as *{const, mut} T" "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)] #[derive(Copy, Clone)]
pub struct Pass; pub struct Pass;
@ -214,7 +236,8 @@ impl LintPass for Pass {
REDUNDANT_PATTERN, REDUNDANT_PATTERN,
USED_UNDERSCORE_BINDING, USED_UNDERSCORE_BINDING,
SHORT_CIRCUIT_STATEMENT, SHORT_CIRCUIT_STATEMENT,
ZERO_PTR ZERO_PTR,
FLOAT_CMP_CONST
) )
} }
} }
@ -334,7 +357,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
return; 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 lhs = Sugg::hir(cx, left, "..");
let rhs = Sugg::hir(cx, right, ".."); let rhs = Sugg::hir(cx, right, "..");
@ -421,6 +449,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 { 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_item = cx.tcx.hir.get_parent(expr.id);
let parent_def_id = cx.tcx.hir.local_def_id(parent_item); let parent_def_id = cx.tcx.hir.local_def_id(parent_item);

View file

@ -0,0 +1,31 @@
#![warn(float_cmp_const)]
#![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;
}

View file

@ -0,0 +1,85 @@
error: strict comparison of f32 or f64 constant
--> $DIR/float_cmp_const.rs:16:5
|
16 | 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:16:5
|
16 | 1f32 == ONE;
| ^^^^^^^^^^^
error: strict comparison of f32 or f64 constant
--> $DIR/float_cmp_const.rs:17:5
|
17 | 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:17:5
|
17 | TWO == 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 | 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:19:5
|
19 | ONE + ONE == TWO;
| ^^^^^^^^^^^^^^^^
error: strict comparison of f32 or f64 constant
--> $DIR/float_cmp_const.rs:20:5
|
20 | 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:20:5
|
20 | 1 as f32 == ONE;
| ^^^^^^^^^^^^^^^
error: strict comparison of f32 or f64 constant
--> $DIR/float_cmp_const.rs:23:5
|
23 | 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:23:5
|
23 | v == 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;
| ^^^^^^^^