From 8adbf7f2c1845cd8a23e1209eef7a0df34137f5a Mon Sep 17 00:00:00 2001 From: Jerry Hardee Date: Mon, 15 Jul 2019 12:46:58 -0500 Subject: [PATCH 1/2] Fix float_cmp false positive when comparing signum f1.signum() == f2.signum() f1.signum() != f2.signum() should not trigger a warning. --- clippy_lints/src/misc.rs | 27 +++++++++++++++++++++++++++ tests/ui/float_cmp.rs | 19 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index acca50e3df88..04c0983db688 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -364,6 +364,12 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints { if is_allowed(cx, left) || is_allowed(cx, right) { return; } + + // Allow comparing the results of signum() + if is_signum(cx, left) && is_signum(cx, right) { + return; + } + if let Some(name) = get_item_name(cx, expr) { let name = name.as_str(); if name == "eq" @@ -493,6 +499,27 @@ fn is_allowed<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> bool { } } +// Return true if `expr` is the result of `signum()` invoked on a float value. +fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { + // The negation of a signum is still a signum + if let ExprKind::Unary(UnNeg, ref child_expr) = expr.node { + return is_signum(cx, &child_expr); + } + + if_chain! { + if let ExprKind::MethodCall(ref method_name, _, ref expressions) = expr.node; + if sym!(signum) == method_name.ident.name; + // Check that the receiver of the signum() is a float (expressions[0] is the receiver of + // the method call) + if is_float(cx, &expressions[0]); + then { + true + } else { + false + } + } +} + fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { matches!(walk_ptrs_ty(cx.tables.expr_ty(expr)).sty, ty::Float(_)) } diff --git a/tests/ui/float_cmp.rs b/tests/ui/float_cmp.rs index 92d38527bbf1..1ec8af3e3871 100644 --- a/tests/ui/float_cmp.rs +++ b/tests/ui/float_cmp.rs @@ -77,4 +77,23 @@ fn main() { let b: *const f32 = xs.as_ptr(); assert_eq!(a, b); // no errors + + // no errors - comparing signums is ok + let x32 = 3.21f32; + 1.23f32.signum() == x32.signum(); + 1.23f32.signum() == -(x32.signum()); + 1.23f32.signum() == 3.21f32.signum(); + + 1.23f32.signum() != x32.signum(); + 1.23f32.signum() != -(x32.signum()); + 1.23f32.signum() != 3.21f32.signum(); + + let x64 = 3.21f64; + 1.23f64.signum() == x64.signum(); + 1.23f64.signum() == -(x64.signum()); + 1.23f64.signum() == 3.21f64.signum(); + + 1.23f64.signum() != x64.signum(); + 1.23f64.signum() != -(x64.signum()); + 1.23f64.signum() != 3.21f64.signum(); } From 8a8eedf388a1613c5a084caa4c52ec520672576e Mon Sep 17 00:00:00 2001 From: Jerry Hardee Date: Mon, 15 Jul 2019 14:00:07 -0500 Subject: [PATCH 2/2] Lint --- clippy_lints/src/misc.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 04c0983db688..d89f886a39b3 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -511,13 +511,11 @@ fn is_signum(cx: &LateContext<'_, '_>, expr: &Expr) -> bool { if sym!(signum) == method_name.ident.name; // Check that the receiver of the signum() is a float (expressions[0] is the receiver of // the method call) - if is_float(cx, &expressions[0]); then { - true - } else { - false + return is_float(cx, &expressions[0]); } } + false } fn is_float(cx: &LateContext<'_, '_>, expr: &Expr) -> bool {