From 5f1bb7216cb901f0cbb5d15dcb1168c0e6c69c8e Mon Sep 17 00:00:00 2001 From: briankabiro Date: Wed, 31 Jul 2019 14:52:12 +0300 Subject: [PATCH 01/15] Add lint when comparing floats in an array Finishes #4277 --- clippy_lints/src/misc.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 6618876b3751..083efd4d681d 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -500,8 +500,14 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { false } -fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { - matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).kind, ty::Float(_)) +fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { + let value = &walk_ptrs_ty(cx.tables.expr_ty(expr)).sty; + + if let ty::Array(arr_ty, _) = value { + return matches!(arr_ty.sty, ty::Float(_)); + }; + + matches!(value, ty::Float(_)) } fn check_to_owned(cx: &LateContext<'_, '_>, expr: &Expr<'_>, other: &Expr<'_>) { From b53bcc7af3667fb40ab1751b186bde2438c985ad Mon Sep 17 00:00:00 2001 From: briankabiro Date: Thu, 19 Sep 2019 15:57:43 +0300 Subject: [PATCH 02/15] Add tests for float in array comparison --- tests/ui/float_cmp.rs | 7 +++++++ tests/ui/float_cmp.stderr | 28 +++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index 207c1bcbbc67..ff3b94c0cef1 100644 --- a/tests/ui/float_cmp.rs +++ b/tests/ui/float_cmp.rs @@ -77,6 +77,13 @@ fn main() { assert_eq!(a, b); // no errors + let a1: [f32; 1] = [0.0]; + let a2: [f32; 1] = [1.1]; + + assert_eq!(a1[0], a2[0]); + + assert_eq!(&a1[0], &a2[0]); + // no errors - comparing signums is ok let x32 = 3.21f32; 1.23f32.signum() == x32.signum(); diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index 68f5b23bdc73..b242ca8fd85b 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -35,5 +35,31 @@ note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error: strict comparison of f32 or f64 + --> $DIR/float_cmp.rs:83:5 + | +LL | assert_eq!(a1[0], a2[0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp.rs:83:5 + | +LL | assert_eq!(a1[0], a2[0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: strict comparison of f32 or f64 + --> $DIR/float_cmp.rs:85:5 + | +LL | assert_eq!(&a1[0], &a2[0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: std::f32::EPSILON and std::f64::EPSILON are available. + --> $DIR/float_cmp.rs:85:5 + | +LL | assert_eq!(&a1[0], &a2[0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to 5 previous errors From e91f5b29bb6ec4429c1f4c1ad8715c0b784b1ed4 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Tue, 17 Mar 2020 09:03:36 +0100 Subject: [PATCH 03/15] Update field names in is_float --- clippy_lints/src/misc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 083efd4d681d..b655febdfc23 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -500,11 +500,11 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { false } -fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { - let value = &walk_ptrs_ty(cx.tables.expr_ty(expr)).sty; +fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { + let value = &walk_ptrs_ty(cx.tables.expr_ty(expr)).kind; if let ty::Array(arr_ty, _) = value { - return matches!(arr_ty.sty, ty::Float(_)); + return matches!(arr_ty.kind, ty::Float(_)); }; matches!(value, ty::Float(_)) From 0f9a0f8f4a7b7b60ec762202f0dd244897cd5c78 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Tue, 17 Mar 2020 10:32:20 +0100 Subject: [PATCH 04/15] Update stderr of float_cmp test --- tests/ui/float_cmp.stderr | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index b242ca8fd85b..c7d278f1261a 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -35,31 +35,31 @@ note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: strict comparison of f32 or f64 +error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:83:5 | LL | assert_eq!(a1[0], a2[0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: std::f32::EPSILON and std::f64::EPSILON are available. +note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. --> $DIR/float_cmp.rs:83:5 | LL | assert_eq!(a1[0], a2[0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: strict comparison of f32 or f64 +error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:85:5 | LL | assert_eq!(&a1[0], &a2[0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: std::f32::EPSILON and std::f64::EPSILON are available. +note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. --> $DIR/float_cmp.rs:85:5 | LL | assert_eq!(&a1[0], &a2[0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 5 previous errors From b6306447e15b7e0e07d15c00d8c8742e67de18e6 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Thu, 19 Mar 2020 15:53:02 +0100 Subject: [PATCH 05/15] Add handling of float arrays to miri_to_const --- clippy_lints/src/consts.rs | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index fc26755a3754..624e109cf03e 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -492,6 +492,41 @@ pub fn miri_to_const(result: &ty::Const<'_>) -> Option { }, _ => 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::>>() + .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::>>() + .map(Constant::Vec), + _ => None, + }, + // FIXME: implement other array type conversions. + _ => None, + }, + _ => None, + }, // FIXME: implement other conversions. _ => None, } From 07660023997d7353f18129221bee1edede048a02 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Thu, 19 Mar 2020 16:54:19 +0100 Subject: [PATCH 06/15] Handle evaluating constant index expression --- clippy_lints/src/consts.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 624e109cf03e..6f62bc1bcbd3 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -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,20 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { } } + fn index(&mut self, lhs: &'_ Expr<'_>, index: &'_ Expr<'_>) -> Option { + 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, + }, + _ => None, + } + } + /// A block can only yield a constant if it only has one constant expression. fn block(&mut self, block: &Block<'_>) -> Option { if block.stmts.is_empty() { From 74f5090c6a9f31c9fd2436de8da01daca57c9356 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Fri, 20 Mar 2020 10:40:44 +0100 Subject: [PATCH 07/15] Allow for const arrays of zeros --- clippy_lints/src/misc.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index b655febdfc23..8419edc20022 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -477,6 +477,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, } } From bce12937c1a871835b6b8760e2f5ddb72711f5a0 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Fri, 20 Mar 2020 10:51:48 +0100 Subject: [PATCH 08/15] Don't show comparison suggestion for arrays --- clippy_lints/src/misc.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 8419edc20022..c9841cf26dc5 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -380,16 +380,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { 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 - ); + if !(is_array(cx, left) || is_array(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, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); }); } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) { @@ -515,6 +517,10 @@ fn is_float(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool { 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<'_>) { let (arg_ty, snip) = match expr.kind { ExprKind::MethodCall(.., ref args) if args.len() == 1 => { From 4165236c811e203acf55d162a1c28d0ba42a3518 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Fri, 20 Mar 2020 11:25:39 +0100 Subject: [PATCH 09/15] Handle constant arrays with single value --- clippy_lints/src/consts.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 6f62bc1bcbd3..1eafa42e7305 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -356,6 +356,17 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { 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, } } From 517fa65c7c37bc853a70c5a680befa52705ba8dc Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Fri, 20 Mar 2020 11:42:39 +0100 Subject: [PATCH 10/15] Add float cmp tests for arrays --- tests/ui/float_cmp.rs | 22 ++++++++++++++--- tests/ui/float_cmp.stderr | 52 +++++++++++++++++++++++---------------- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index ff3b94c0cef1..eb2f532c7726 100644 --- a/tests/ui/float_cmp.rs +++ b/tests/ui/float_cmp.rs @@ -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,12 +83,20 @@ 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]; - assert_eq!(a1[0], a2[0]); - - assert_eq!(&a1[0], &a2[0]); + a1 == a2; + a1[0] == a2[0]; // no errors - comparing signums is ok let x32 = 3.21f32; diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index c7d278f1261a..e4df2b78b0ba 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -1,65 +1,75 @@ 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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:59:5 + --> $DIR/float_cmp.rs:65:5 | LL | ONE as f64 != 2.0; | ^^^^^^^^^^^^^^^^^ 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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:64:5 + --> $DIR/float_cmp.rs:70:5 | LL | x == 1.0; | ^^^^^^^^ 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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:67:5 + --> $DIR/float_cmp.rs:73:5 | LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:83:5 + --> $DIR/float_cmp.rs:93:5 | -LL | assert_eq!(a1[0], a2[0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ +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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:83:5 + --> $DIR/float_cmp.rs:93:5 | -LL | assert_eq!(a1[0], a2[0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) +LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:85:5 + --> $DIR/float_cmp.rs:98:5 | -LL | assert_eq!(&a1[0], &a2[0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | a1 == a2; + | ^^^^^^^^ + | +note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. + --> $DIR/float_cmp.rs:98:5 + | +LL | a1 == a2; + | ^^^^^^^^ + +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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:85:5 + --> $DIR/float_cmp.rs:99:5 | -LL | assert_eq!(&a1[0], &a2[0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) +LL | a1[0] == a2[0]; + | ^^^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors From 1a7bddb8d178f3dd20fdb4998244abc9b5b24fe1 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Fri, 20 Mar 2020 11:54:04 +0100 Subject: [PATCH 11/15] Add float cmp const tests for arrays --- tests/ui/float_cmp_const.rs | 13 +++++++++++++ tests/ui/float_cmp_const.stderr | 14 +++++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/tests/ui/float_cmp_const.rs b/tests/ui/float_cmp_const.rs index 8f4ad15720b0..ddf6a9e39f64 100644 --- a/tests/ui/float_cmp_const.rs +++ b/tests/ui/float_cmp_const.rs @@ -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; } diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr index c13c555cd119..9bdfa24d97e7 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -83,5 +83,17 @@ note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. LL | v != ONE; | ^^^^^^^^ -error: aborting due to 7 previous errors +error: strict comparison of `f32` or `f64` constant + --> $DIR/float_cmp_const.rs:61:5 + | +LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. + --> $DIR/float_cmp_const.rs:61:5 + | +LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 8 previous errors From f8fd4aca783bf52c4f6c41a6b38549e41db2d1d3 Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Mon, 6 Apr 2020 08:56:22 +0200 Subject: [PATCH 12/15] Make epsilon note spanless when comparing arrays --- clippy_lints/src/misc.rs | 4 +++- tests/ui/float_cmp.stderr | 6 +----- tests/ui/float_cmp_const.stderr | 6 +----- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index c9841cf26dc5..a60d67ee43f7 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -391,8 +391,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { ), Applicability::HasPlaceholders, // snippet ); + db.span_note(expr.span, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); + } else { + db.note("`std::f32::EPSILON` and `std::f64::EPSILON` are available."); } - db.span_note(expr.span, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); }); } 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"); diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index e4df2b78b0ba..20c45f3fcb6d 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -53,11 +53,7 @@ error: strict comparison of `f32` or `f64` LL | a1 == a2; | ^^^^^^^^ | -note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:98:5 - | -LL | a1 == a2; - | ^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:99:5 diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr index 9bdfa24d97e7..f07f0d35ab09 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -89,11 +89,7 @@ error: strict comparison of `f32` or `f64` constant LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | -note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:61:5 - | -LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. error: aborting due to 8 previous errors From 7294acba5a45af1ddcbaab3c1eb90334af99256a Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Mon, 6 Apr 2020 09:06:50 +0200 Subject: [PATCH 13/15] Indicate when arrays are compared in error message --- clippy_lints/src/misc.rs | 21 ++++++++++++++++++--- tests/ui/float_cmp.stderr | 2 +- tests/ui/float_cmp_const.stderr | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index a60d67ee43f7..93616b864432 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -371,16 +371,31 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { return; } } + let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); let (lint, msg) = if is_named_constant(cx, left) || is_named_constant(cx, right) { - (FLOAT_CMP_CONST, "strict comparison of `f32` or `f64` constant") + ( + 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, "strict comparison of `f32` or `f64`") + ( + FLOAT_CMP, + if is_comparing_arrays { + "strict comparison of `f32` or `f64` arrays" + } else { + "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, ".."); - if !(is_array(cx, left) || is_array(cx, right)) { + if !is_comparing_arrays { db.span_suggestion( expr.span, "consider comparing them within some error", diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index 20c45f3fcb6d..9775527c9055 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -47,7 +47,7 @@ note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: strict comparison of `f32` or `f64` +error: strict comparison of `f32` or `f64` arrays --> $DIR/float_cmp.rs:98:5 | LL | a1 == a2; diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr index f07f0d35ab09..218ea876c579 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -83,7 +83,7 @@ note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. LL | v != ONE; | ^^^^^^^^ -error: strict comparison of `f32` or `f64` constant +error: strict comparison of `f32` or `f64` constant arrays --> $DIR/float_cmp_const.rs:61:5 | LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; From 16b4003c0c2949df32c9b2cb0d43348bd337658b Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Mon, 6 Apr 2020 09:40:53 +0200 Subject: [PATCH 14/15] Split check_fn function --- clippy_lints/src/misc.rs | 54 ++++++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 93616b864432..13dbc5017b8a 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -372,30 +372,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { } } let is_comparing_arrays = is_array(cx, left) || is_array(cx, right); - let (lint, msg) = if is_named_constant(cx, left) || is_named_constant(cx, right) { - ( - 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`" - }, - ) - }; + 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, ".."); - if !is_comparing_arrays { + if is_comparing_arrays { + db.note("`std::f32::EPSILON` and `std::f64::EPSILON` are available."); + } else { db.span_suggestion( expr.span, "consider comparing them within some error", @@ -407,8 +394,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { Applicability::HasPlaceholders, // snippet ); db.span_note(expr.span, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); - } else { - db.note("`std::f32::EPSILON` and `std::f64::EPSILON` are available."); } }); } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) { @@ -461,6 +446,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); From 4348af2b20bddf7e90aab3628977f900f73580ca Mon Sep 17 00:00:00 2001 From: Marcin Serwin Date: Mon, 6 Apr 2020 15:29:54 +0200 Subject: [PATCH 15/15] Make the epsilon note spanless --- clippy_lints/src/misc.rs | 6 ++--- tests/ui/float_cmp.stderr | 32 +++++------------------- tests/ui/float_cmp_const.stderr | 44 ++++++--------------------------- 3 files changed, 16 insertions(+), 66 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 13dbc5017b8a..56fd65e7c296 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -380,9 +380,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { let lhs = Sugg::hir(cx, left, ".."); let rhs = Sugg::hir(cx, right, ".."); - if is_comparing_arrays { - db.note("`std::f32::EPSILON` and `std::f64::EPSILON` are available."); - } else { + if !is_comparing_arrays { db.span_suggestion( expr.span, "consider comparing them within some error", @@ -393,8 +391,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { ), Applicability::HasPlaceholders, // snippet ); - db.span_note(expr.span, "`std::f32::EPSILON` and `std::f64::EPSILON` are available."); } + db.note("`std::f32::EPSILON` and `std::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"); diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index 9775527c9055..277e10e17ad9 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -5,11 +5,7 @@ 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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:65:5 - | -LL | ONE as f64 != 2.0; - | ^^^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:70:5 @@ -17,11 +13,7 @@ error: strict comparison of `f32` or `f64` LL | x == 1.0; | ^^^^^^^^ help: consider comparing them within some error: `(x - 1.0).abs() < error` | -note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:70:5 - | -LL | x == 1.0; - | ^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:73:5 @@ -29,11 +21,7 @@ error: strict comparison of `f32` or `f64` LL | twice(x) != twice(ONE as f64); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider comparing them within some error: `(twice(x) - twice(ONE as f64)).abs() > error` | -note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:73:5 - | -LL | twice(x) != twice(ONE as f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:93:5 @@ -41,11 +29,7 @@ error: strict comparison of `f32` or `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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:93:5 - | -LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` arrays --> $DIR/float_cmp.rs:98:5 @@ -53,7 +37,7 @@ error: strict comparison of `f32` or `f64` arrays LL | a1 == a2; | ^^^^^^^^ | - = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` --> $DIR/float_cmp.rs:99:5 @@ -61,11 +45,7 @@ error: strict comparison of `f32` or `f64` LL | a1[0] == a2[0]; | ^^^^^^^^^^^^^^ help: consider comparing them within some error: `(a1[0] - a2[0]).abs() < error` | -note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp.rs:99:5 - | -LL | a1[0] == a2[0]; - | ^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: aborting due to 6 previous errors diff --git a/tests/ui/float_cmp_const.stderr b/tests/ui/float_cmp_const.stderr index 218ea876c579..15bd11b04faa 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:20:5 - | -LL | 1f32 == ONE; - | ^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:21:5 - | -LL | TWO == ONE; - | ^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:22:5 - | -LL | TWO != ONE; - | ^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:23:5 - | -LL | ONE + ONE == TWO; - | ^^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:25:5 - | -LL | x as f32 == ONE; - | ^^^^^^^^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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: `std::f32::EPSILON` and `std::f64::EPSILON` are available. - --> $DIR/float_cmp_const.rs:28:5 - | -LL | v == ONE; - | ^^^^^^^^ + = note: `std::f32::EPSILON` and `std::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,7 @@ error: strict comparison of `f32` or `f64` constant LL | 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:29:5 - | -LL | v != ONE; - | ^^^^^^^^ + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: strict comparison of `f32` or `f64` constant arrays --> $DIR/float_cmp_const.rs:61:5 @@ -89,7 +61,7 @@ error: strict comparison of `f32` or `f64` constant arrays LL | NON_ZERO_ARRAY == NON_ZERO_ARRAY2; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available. + = note: `std::f32::EPSILON` and `std::f64::EPSILON` are available for the `error` error: aborting due to 8 previous errors