diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 810c6e1412a..970aeef623e 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -6,7 +6,7 @@ use crate::utils::{higher, span_lint_and_sugg, sugg, SpanlessEq}; use if_chain::if_chain; use rustc::ty; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind, Lit, UnOp}; +use rustc_hir::{BinOpKind, Block, Expr, ExprKind, Lit, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::source_map::Spanned; @@ -409,22 +409,46 @@ fn is_zero(expr: &Expr<'_>) -> bool { } fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) { -// if let Some((cond, body, Some(else_body))) = higher::if_block(&expr) { - // Check for the positive-first variant -// if let ExprKind::Unary(UnOp::UnNeg, expr) = else_body.kind { -// if are_exprs_equal(cx, expr, body) && is_testing_positive(cx, cond, body) { - span_lint_and_sugg( - cx, - SUBOPTIMAL_FLOPS, - expr.span, - "This looks like you've implemented your own absolute value function", - "try", - "a.abs()".to_string(),//format!("{:?}.abs()", body), - Applicability::MachineApplicable, - ); -// } -// } -// } + if let Some((cond, body, Some(else_body))) = higher::if_block(&expr) { + if let ExprKind::Block( + Block { + stmts: [], + expr: + Some(Expr { + kind: ExprKind::Unary(UnOp::UnNeg, else_expr), + .. + }), + .. + }, + _, + ) = else_body.kind + { + if let ExprKind::Block( + Block { + stmts: [], + expr: Some(body), + .. + }, + _, + ) = &body.kind + { + if are_exprs_equal(cx, else_expr, body) { + dbg!("if (cond) body else -body\nbody: {:?}", &body.kind); + if is_testing_positive(cx, cond, body) { + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "This looks like you've implemented your own absolute value function", + "try", + format!("{}.abs()", Sugg::hir(cx, body, "..")), + Applicability::MachineApplicable, + ); + } + } + } + } + } } impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic { diff --git a/tests/ui/floating_point_abs.rs b/tests/ui/floating_point_abs.rs index 2cb1db09541..0efc7092899 100644 --- a/tests/ui/floating_point_abs.rs +++ b/tests/ui/floating_point_abs.rs @@ -1,4 +1,8 @@ #[warn(clippy::suboptimal_flops)] +struct A { + a: f64, + b: f64 +} fn fake_abs1(num: f64) -> f64 { if num >= 0.0 { @@ -16,6 +20,14 @@ fn fake_abs2(num: f64) -> f64 { } } +fn fake_abs3(a: A) -> f64 { + if a.a > 0.0 { + a.a + } else { + -a.a + } +} + fn fake_nabs1(num: f64) -> f64 { if num < 0.0 { num @@ -32,6 +44,14 @@ fn fake_nabs2(num: f64) -> f64 { } } +fn fake_nabs3(a: A) -> A { + A { a: if a.a >= 0.0 { + a.a + } else { + -a.a + }, b: a.b } +} + fn not_fake_abs1(num: f64) -> f64 { if num > 0.0 { num @@ -56,4 +76,20 @@ fn not_fake_abs3(num1: f64, num2: f64) -> f64 { } } +fn not_fake_abs4(a: A) -> f64 { + if a.a > 0.0 { + a.b + } else { + -a.b + } +} + +fn not_fake_abs5(a: A) -> f64 { + if a.a > 0.0 { + a.a + } else { + -a.b + } +} + fn main() {} diff --git a/tests/ui/floating_point_abs.stderr b/tests/ui/floating_point_abs.stderr new file mode 100644 index 00000000000..bbd67de17c0 --- /dev/null +++ b/tests/ui/floating_point_abs.stderr @@ -0,0 +1,14 @@ +error: This looks like you've implemented your own absolute value function + --> $DIR/floating_point_abs.rs:4:5 + | +LL | / if num >= 0.0 { +LL | | num +LL | | } else { +LL | | -num +LL | | } + | |_____^ help: try: `num.abs()` + | + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` + +error: aborting due to previous error +