Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-10 15:14:29 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #5345 - Toxyxer:add-lint-for-float-in-array-comparison, r=flip1995

Browse source 
Add lint for float in array comparison

Fixes #4277
changelog:
- Added new handler for expression of index kind (e.g. `arr[i]`). It returns a constant when both array and index are constant, or when the array is constant and all values are equal.
- Trigger float_cmp and float_cmp_const lint when comparing arrays. Allow for comparison when one of the arrays contains only zeros or infinities.
- Added appropriate tests for such cases.
This commit is contained in:
bors 2020-04-15 06:39:11 +00:00
commit a96f874301
6 changed files with 200 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

61
clippy_lints/src/consts.rs
View file

@ -268,6 +268,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
}
}
},
ExprKind::Index(ref arr, ref index) => self.index(arr, index),
// TODO: add other expressions.
_ => None,
}
@ -345,6 +346,31 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
}
}
fn index(&mut self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option<Constant> {
let lhs = self.expr(lhs);
let index = self.expr(index);
match (lhs, index) {
(Some(Constant::Vec(vec)), Some(Constant::Int(index))) => match vec[index as usize] {
Constant::F32(x) => Some(Constant::F32(x)),
Constant::F64(x) => Some(Constant::F64(x)),
_ => None,
},
(Some(Constant::Vec(vec)), _) => {
if !vec.is_empty() && vec.iter().all(|x| *x == vec[0]) {
match vec[0] {
Constant::F32(x) => Some(Constant::F32(x)),
Constant::F64(x) => Some(Constant::F64(x)),
_ => None,
}
} else {
None
}
},
_ => None,
}
}
/// A block can only yield a constant if it only has one constant expression.
fn block(&mut self, block: &Block<'_>) -> Option<Constant> {
if block.stmts.is_empty() {
@ -492,6 +518,41 @@ pub fn miri_to_const(result: &ty::Const<'_>) -> Option<Constant> {
},
_ => None,
},
ty::ConstKind::Value(ConstValue::ByRef { alloc, offset: _ }) => match result.ty.kind {
ty::Array(sub_type, len) => match sub_type.kind {
ty::Float(FloatTy::F32) => match miri_to_const(len) {
Some(Constant::Int(len)) => alloc
.inspect_with_undef_and_ptr_outside_interpreter(0..(4 * len as usize))
.to_owned()
.chunks(4)
.map(|chunk| {
Some(Constant::F32(f32::from_le_bytes(
chunk.try_into().expect("this shouldn't happen"),
)))
})
.collect::<Option<Vec<Constant>>>()
.map(Constant::Vec),
_ => None,
},
ty::Float(FloatTy::F64) => match miri_to_const(len) {
Some(Constant::Int(len)) => alloc
.inspect_with_undef_and_ptr_outside_interpreter(0..(8 * len as usize))
.to_owned()
.chunks(8)
.map(|chunk| {
Some(Constant::F64(f64::from_le_bytes(
chunk.try_into().expect("this shouldn't happen"),
)))
})
.collect::<Option<Vec<Constant>>>()
.map(Constant::Vec),
_ => None,
},
// FIXME: implement other array type conversions.
_ => None,
},
_ => None,
},
// FIXME: implement other conversions.
_ => None,
}

76
clippy_lints/src/misc.rs
View file

@ -369,26 +369,28 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
return;
}
}
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`")
};
let is_comparing_arrays = is_array(cx, left) || is_array(cx, right);
let (lint, msg) = get_lint_and_message(
is_named_constant(cx, left) || is_named_constant(cx, right),
is_comparing_arrays,
);
span_lint_and_then(cx, lint, expr.span, msg, |db| {
let lhs = Sugg::hir(cx, left, "..");
let rhs = Sugg::hir(cx, right, "..");
db.span_suggestion(
expr.span,
"consider comparing them within some error",
format!(
"({}).abs() {} error",
lhs - rhs,
if op == BinOpKind::Eq { '<' } else { '>' }
),
Applicability::HasPlaceholders, // snippet
);
db.span_note(expr.span, "`f32::EPSILON` and `f64::EPSILON` are available.");
if !is_comparing_arrays {
db.span_suggestion(
expr.span,
"consider comparing them within some error",
format!(
"({}).abs() {} error",
lhs - rhs,
if op == BinOpKind::Eq { '<' } else { '>' }
),
Applicability::HasPlaceholders, // snippet
);
}
db.note("`f32::EPSILON` and `f64::EPSILON` are available for the `error`");
});
} else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) {
span_lint(cx, MODULO_ONE, expr.span, "any number modulo 1 will be 0");
@ -440,6 +442,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
}
}
fn get_lint_and_message(
is_comparing_constants: bool,
is_comparing_arrays: bool,
) -> (&'static rustc_lint::Lint, &'static str) {
if is_comparing_constants {
(
FLOAT_CMP_CONST,
if is_comparing_arrays {
"strict comparison of `f32` or `f64` constant arrays"
} else {
"strict comparison of `f32` or `f64` constant"
},
)
} else {
(
FLOAT_CMP,
if is_comparing_arrays {
"strict comparison of `f32` or `f64` arrays"
} else {
"strict comparison of `f32` or `f64`"
},
)
}
}
fn check_nan(cx: &LateContext<'_, '_>, expr: &Expr<'_>, cmp_expr: &Expr<'_>) {
if_chain! {
if !in_constant(cx, cmp_expr.hir_id);
@ -475,6 +502,11 @@ fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) -> boo
match constant(cx, cx.tables, expr) {
Some((Constant::F32(f), _)) => f == 0.0 || f.is_infinite(),
Some((Constant::F64(f), _)) => f == 0.0 || f.is_infinite(),
Some((Constant::Vec(vec), _)) => vec.iter().all(|f| match f {
Constant::F32(f) => *f == 0.0 || (*f).is_infinite(),
Constant::F64(f) => *f == 0.0 || (*f).is_infinite(),
_ => false,
}),
_ => false,
}
}
@ -499,7 +531,17 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
}
fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).kind, ty::Float(_))
let value = &walk_ptrs_ty(cx.tables.expr_ty(expr)).kind;
if let ty::Array(arr_ty, _) = value {
return matches!(arr_ty.kind, ty::Float(_));
};
matches!(value, ty::Float(_))
}
fn is_array(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
matches!(&walk_ptrs_ty(cx.tables.expr_ty(expr)).kind, ty::Array(_, _))
}
fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) {

23
tests/ui/float_cmp.rs
View file

@ -1,5 +1,11 @@
#![warn(clippy::float_cmp)]
#![allow(unused, clippy::no_effect, clippy::unnecessary_operation, clippy::cast_lossless)]
#![allow(
unused,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::cast_lossless,
clippy::many_single_char_names
)]
use std::ops::Add;
@ -77,6 +83,21 @@ fn main() {
assert_eq!(a, b); // no errors
const ZERO_ARRAY: [f32; 2] = [0.0, 0.0];
const NON_ZERO_ARRAY: [f32; 2] = [0.0, 0.1];
let i = 0;
let j = 1;
ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; // ok, because lhs is zero regardless of i
NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j];
let a1: [f32; 1] = [0.0];
let a2: [f32; 1] = [1.1];
a1 == a2;
a1[0] == a2[0];
// no errors - comparing signums is ok
let x32 = 3.21f32;
1.23f32.signum() == x32.signum();

48
tests/ui/float_cmp.stderr
View file

@ -1,39 +1,51 @@
error: strict comparison of `f32` or `f64`
--> $DIR/float_cmp.rs:59:5
--> $DIR/float_cmp.rs:65:5
|
LL | ONE as f64 != 2.0;
| ^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE as f64 - 2.0).abs() > error`
|
= note: `-D clippy::float-cmp` implied by `-D warnings`
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp.rs:59:5
|
LL | ONE as f64 != 2.0;
| ^^^^^^^^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64`
--> $DIR/float_cmp.rs:64:5
--> $DIR/float_cmp.rs:70:5
|
LL | x == 1.0;
| ^^^^^^^^ help: consider comparing them within some error: `(x - 1.0).abs() < error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp.rs:64:5
|
LL | x == 1.0;
| ^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64`
--> $DIR/float_cmp.rs:67:5
--> $DIR/float_cmp.rs:73:5
|
LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(twice(x) - twice(ONE as f64)).abs() > error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp.rs:67:5
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64`
--> $DIR/float_cmp.rs:93:5
|
LL | twice(x) != twice(ONE as f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(NON_ZERO_ARRAY[i] - NON_ZERO_ARRAY[j]).abs() < error`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: aborting due to 3 previous errors
error: strict comparison of `f32` or `f64` arrays
--> $DIR/float_cmp.rs:98:5
|
LL | a1 == a2;
| ^^^^^^^^
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64`
--> $DIR/float_cmp.rs:99:5
|
LL | a1[0] == a2[0];
| ^^^^^^^^^^^^^^ help: consider comparing them within some error: `(a1[0] - a2[0]).abs() < error`
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: aborting due to 6 previous errors

13
tests/ui/float_cmp_const.rs
View file

@ -46,4 +46,17 @@ fn main() {
v != w;
v == 1.0;
v != 1.0;
const ZERO_ARRAY: [f32; 3] = [0.0, 0.0, 0.0];
const ZERO_INF_ARRAY: [f32; 3] = [0.0, ::std::f32::INFINITY, ::std::f32::NEG_INFINITY];
const NON_ZERO_ARRAY: [f32; 3] = [0.0, 0.1, 0.2];
const NON_ZERO_ARRAY2: [f32; 3] = [0.2, 0.1, 0.0];
// no errors, zero and infinity values
NON_ZERO_ARRAY[0] == NON_ZERO_ARRAY2[1]; // lhs is 0.0
ZERO_ARRAY == NON_ZERO_ARRAY; // lhs is all zeros
ZERO_INF_ARRAY == NON_ZERO_ARRAY; // lhs is all zeros or infinities
// has errors
NON_ZERO_ARRAY == NON_ZERO_ARRAY2;
}

50
tests/ui/float_cmp_const.stderr
View file

@ -5,11 +5,7 @@ LL | 1f32 == ONE;
| ^^^^^^^^^^^ help: consider comparing them within some error: `(1f32 - ONE).abs() < error`
|
= note: `-D clippy::float-cmp-const` implied by `-D warnings`
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:20:5
|
LL | 1f32 == ONE;
| ^^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:21:5
@ -17,11 +13,7 @@ error: strict comparison of `f32` or `f64` constant
LL | TWO == ONE;
| ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() < error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:21:5
|
LL | TWO == ONE;
| ^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:22:5
@ -29,11 +21,7 @@ error: strict comparison of `f32` or `f64` constant
LL | TWO != ONE;
| ^^^^^^^^^^ help: consider comparing them within some error: `(TWO - ONE).abs() > error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:22:5
|
LL | TWO != ONE;
| ^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:23:5
@ -41,11 +29,7 @@ error: strict comparison of `f32` or `f64` constant
LL | ONE + ONE == TWO;
| ^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(ONE + ONE - TWO).abs() < error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:23:5
|
LL | ONE + ONE == TWO;
| ^^^^^^^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:25:5
@ -53,11 +37,7 @@ error: strict comparison of `f32` or `f64` constant
LL | x as f32 == ONE;
| ^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(x as f32 - ONE).abs() < error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:25:5
|
LL | x as f32 == ONE;
| ^^^^^^^^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:28:5
@ -65,11 +45,7 @@ error: strict comparison of `f32` or `f64` constant
LL | v == ONE;
| ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() < error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:28:5
|
LL | v == ONE;
| ^^^^^^^^
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant
--> $DIR/float_cmp_const.rs:29:5
@ -77,11 +53,15 @@ error: strict comparison of `f32` or `f64` constant
LL | v != ONE;
| ^^^^^^^^ help: consider comparing them within some error: `(v - ONE).abs() > error`
|
note: `f32::EPSILON` and `f64::EPSILON` are available.
--> $DIR/float_cmp_const.rs:29:5
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: strict comparison of `f32` or `f64` constant arrays
--> $DIR/float_cmp_const.rs:61:5
|
LL | v != ONE;
| ^^^^^^^^
LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `f32::EPSILON` and `f64::EPSILON` are available for the `error`
error: aborting due to 7 previous errors
error: aborting due to 8 previous errors