From d4409350dc38a4ee22f86ff39f5613b37a2c5771 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 cedd15e8daf6..2c4515260233 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -498,8 +498,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 0d00eafd3297cfd67a1af3ff053bab8ef5f1ca12 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 c8248723bc9d..b5c6a6449c2a 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 90c25a6db37d..7a55aeed3a3b 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -35,5 +35,31 @@ note: `f32::EPSILON` and `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 03f584c0c7b14d6f6effbe0b9e6b4fb00be57fbd 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 2c4515260233..d4a74a6b4515 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -498,11 +498,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 b4ff774d723baa39de0df43a7e87b1b6a1df5a5a 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 7a55aeed3a3b..cb74fac7249e 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -35,31 +35,31 @@ note: `f32::EPSILON` and `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 2153abb4124fd3dca018d4adb4e79693f1a9fedd 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 b1d540c97515..c64c00134e89 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 621767136eae13b2d9ebd462d3b33d6f1ad6b4e9 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 c64c00134e89..c25200c33388 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 bcbb9d9acb43f5b3f1ffe665f9c66d9931e1cc01 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 d4a74a6b4515..50d0830757db 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -475,6 +475,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 1bab67c72b50c995c7b981fddff89e61c8a723af 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 50d0830757db..d9cef4851f53 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -378,16 +378,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, "`f32::EPSILON` and `f64::EPSILON` are available."); }); } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) { @@ -513,6 +515,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 d3167c63f84fcf34e81eb78c44c8f70ada07080b 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 c25200c33388..b91607129158 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 3c738b22869c50120eb8b7806311110ebf8fa870 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 | 56 +++++++++++++++++++++++---------------- 2 files changed, 51 insertions(+), 27 deletions(-) diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index b5c6a6449c2a..9fa0e5f5c079 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 cb74fac7249e..b0d8dd7c3f31 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: `f32::EPSILON` and `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: `f32::EPSILON` and `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: `f32::EPSILON` and `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 +note: `f32::EPSILON` and `f64::EPSILON` are available. + --> $DIR/float_cmp.rs:93:5 + | +LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: strict comparison of `f32` or `f64` + --> $DIR/float_cmp.rs:98:5 + | +LL | a1 == a2; + | ^^^^^^^^ + | +note: `f32::EPSILON` and `f64::EPSILON` are available. + --> $DIR/float_cmp.rs:98: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 == a2; + | ^^^^^^^^ error: strict comparison of `f32` or `f64` - --> $DIR/float_cmp.rs:85:5 + --> $DIR/float_cmp.rs:99:5 | -LL | assert_eq!(&a1[0], &a2[0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +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 +note: `f32::EPSILON` and `f64::EPSILON` are available. + --> $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 c7b5e30423169f7de0e1874c018f47ba4f63d79e 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 a338040e19be..dfc025558a2f 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 2dc43cf4e5fb..da4b0b937a88 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -83,5 +83,17 @@ note: `f32::EPSILON` and `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 84ae3d8bc85453601066bbebba3c372df763773f 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 d9cef4851f53..491cebc96ab0 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -389,8 +389,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { ), Applicability::HasPlaceholders, // snippet ); + db.span_note(expr.span, "`f32::EPSILON` and `f64::EPSILON` are available."); + } else { + db.note("`f32::EPSILON` and `f64::EPSILON` are available."); } - db.span_note(expr.span, "`f32::EPSILON` and `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 b0d8dd7c3f31..8952caa06762 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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:98:5 - | -LL | a1 == a2; - | ^^^^^^^^ + = note: `f32::EPSILON` and `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 da4b0b937a88..f93ee310785e 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 f637c45f8b375606fc077b4a90ef4cb35758d219 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 491cebc96ab0..672fbd360d50 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -369,16 +369,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 8952caa06762..8718cd830276 100644 --- a/tests/ui/float_cmp.stderr +++ b/tests/ui/float_cmp.stderr @@ -47,7 +47,7 @@ note: `f32::EPSILON` and `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 f93ee310785e..5cdbc1d0013b 100644 --- a/tests/ui/float_cmp_const.stderr +++ b/tests/ui/float_cmp_const.stderr @@ -83,7 +83,7 @@ note: `f32::EPSILON` and `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 4e01ca35a08a104b5ab28e3e0c5c324264989fc6 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 672fbd360d50..8d10c22c5030 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -370,30 +370,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", @@ -405,8 +392,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { Applicability::HasPlaceholders, // snippet ); db.span_note(expr.span, "`f32::EPSILON` and `f64::EPSILON` are available."); - } else { - db.note("`f32::EPSILON` and `f64::EPSILON` are available."); } }); } else if op == BinOpKind::Rem && is_integer_const(cx, right, 1) { @@ -459,6 +444,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 4449cc799b737327b773b900f3f9e3684373aaf5 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 8d10c22c5030..58a5a29eb16b 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -378,9 +378,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", @@ -391,8 +389,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { ), Applicability::HasPlaceholders, // snippet ); - db.span_note(expr.span, "`f32::EPSILON` and `f64::EPSILON` are available."); } + 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"); diff --git a/tests/ui/float_cmp.stderr b/tests/ui/float_cmp.stderr index 8718cd830276..2d454e8e70de 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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:65: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: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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:70: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: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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:73:5 - | -LL | twice(x) != twice(ONE as f64); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `f32::EPSILON` and `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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:93:5 - | -LL | NON_ZERO_ARRAY[i] == NON_ZERO_ARRAY[j]; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: `f32::EPSILON` and `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: `f32::EPSILON` and `f64::EPSILON` are available. + = note: `f32::EPSILON` and `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: `f32::EPSILON` and `f64::EPSILON` are available. - --> $DIR/float_cmp.rs:99:5 - | -LL | a1[0] == a2[0]; - | ^^^^^^^^^^^^^^ + = note: `f32::EPSILON` and `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 5cdbc1d0013b..19dc4a284b72 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: `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,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:29:5 - | -LL | v != ONE; - | ^^^^^^^^ + = 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 @@ -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: `f32::EPSILON` and `f64::EPSILON` are available for the `error` error: aborting due to 8 previous errors