From 7747032b774de25cb9e1cf672a9244f17eb234cb Mon Sep 17 00:00:00 2001 From: Elliot Bobrow Date: Sun, 2 Oct 2022 21:00:51 -0700 Subject: [PATCH] `suboptimal_flops` lint for multiply and subtract --- clippy_lints/src/floating_point_arithmetic.rs | 48 ++++++++++++++----- tests/ui/floating_point_mul_add.fixed | 2 + tests/ui/floating_point_mul_add.rs | 2 + tests/ui/floating_point_mul_add.stderr | 30 ++++++++---- tests/ui/floating_point_powi.fixed | 2 + tests/ui/floating_point_powi.rs | 2 + tests/ui/floating_point_powi.stderr | 20 ++++++-- 7 files changed, 81 insertions(+), 25 deletions(-) diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index e71afec12a7..0ed30196475 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -311,7 +311,8 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: if let ExprKind::Binary( Spanned { - node: BinOpKind::Add, .. + node: op @ (BinOpKind::Add | BinOpKind::Sub), + .. }, lhs, rhs, @@ -319,6 +320,16 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: { let other_addend = if lhs.hir_id == expr.hir_id { rhs } else { lhs }; + // Negate expr if original code has subtraction and expr is on the right side + let maybe_neg_sugg = |expr, hir_id| { + let sugg = Sugg::hir(cx, expr, ".."); + if matches!(op, BinOpKind::Sub) && hir_id == rhs.hir_id { + format!("-{sugg}") + } else { + sugg.to_string() + } + }; + span_lint_and_sugg( cx, SUBOPTIMAL_FLOPS, @@ -328,8 +339,8 @@ fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, args: format!( "{}.mul_add({}, {})", Sugg::hir(cx, receiver, "..").maybe_par(), - Sugg::hir(cx, receiver, ".."), - Sugg::hir(cx, other_addend, ".."), + maybe_neg_sugg(receiver, expr.hir_id), + maybe_neg_sugg(other_addend, other_addend.hir_id), ), Applicability::MachineApplicable, ); @@ -443,7 +454,8 @@ fn is_float_mul_expr<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(&' fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { if let ExprKind::Binary( Spanned { - node: BinOpKind::Add, .. + node: op @ (BinOpKind::Add | BinOpKind::Sub), + .. }, lhs, rhs, @@ -457,10 +469,27 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { } } + let maybe_neg_sugg = |expr| { + let sugg = Sugg::hir(cx, expr, ".."); + if let BinOpKind::Sub = op { + format!("-{sugg}") + } else { + sugg.to_string() + } + }; + let (recv, arg1, arg2) = if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, lhs) { - (inner_lhs, inner_rhs, rhs) + ( + inner_lhs, + Sugg::hir(cx, inner_rhs, "..").to_string(), + maybe_neg_sugg(rhs), + ) } else if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, rhs) { - (inner_lhs, inner_rhs, lhs) + ( + inner_lhs, + maybe_neg_sugg(inner_rhs), + Sugg::hir(cx, lhs, "..").to_string(), + ) } else { return; }; @@ -471,12 +500,7 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { expr.span, "multiply and add expressions can be calculated more efficiently and accurately", "consider using", - format!( - "{}.mul_add({}, {})", - prepare_receiver_sugg(cx, recv), - Sugg::hir(cx, arg1, ".."), - Sugg::hir(cx, arg2, ".."), - ), + format!("{}.mul_add({arg1}, {arg2})", prepare_receiver_sugg(cx, recv)), Applicability::MachineApplicable, ); } diff --git a/tests/ui/floating_point_mul_add.fixed b/tests/ui/floating_point_mul_add.fixed index 169ec02f82b..d3e536ba350 100644 --- a/tests/ui/floating_point_mul_add.fixed +++ b/tests/ui/floating_point_mul_add.fixed @@ -19,7 +19,9 @@ fn main() { let d: f64 = 0.0001; let _ = a.mul_add(b, c); + let _ = a.mul_add(b, -c); let _ = a.mul_add(b, c); + let _ = a.mul_add(-b, c); let _ = 2.0f64.mul_add(4.0, a); let _ = 2.0f64.mul_add(4., a); diff --git a/tests/ui/floating_point_mul_add.rs b/tests/ui/floating_point_mul_add.rs index 5338d4fc2b7..5d4a9e35cfc 100644 --- a/tests/ui/floating_point_mul_add.rs +++ b/tests/ui/floating_point_mul_add.rs @@ -19,7 +19,9 @@ fn main() { let d: f64 = 0.0001; let _ = a * b + c; + let _ = a * b - c; let _ = c + a * b; + let _ = c - a * b; let _ = a + 2.0 * 4.0; let _ = a + 2. * 4.; diff --git a/tests/ui/floating_point_mul_add.stderr b/tests/ui/floating_point_mul_add.stderr index e637bbf90ca..a79ae94e8d4 100644 --- a/tests/ui/floating_point_mul_add.stderr +++ b/tests/ui/floating_point_mul_add.stderr @@ -9,56 +9,68 @@ LL | let _ = a * b + c; error: multiply and add expressions can be calculated more efficiently and accurately --> $DIR/floating_point_mul_add.rs:22:13 | +LL | let _ = a * b - c; + | ^^^^^^^^^ help: consider using: `a.mul_add(b, -c)` + +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_mul_add.rs:23:13 + | LL | let _ = c + a * b; | ^^^^^^^^^ help: consider using: `a.mul_add(b, c)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:23:13 + --> $DIR/floating_point_mul_add.rs:24:13 + | +LL | let _ = c - a * b; + | ^^^^^^^^^ help: consider using: `a.mul_add(-b, c)` + +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_mul_add.rs:25:13 | LL | let _ = a + 2.0 * 4.0; | ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4.0, a)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:24:13 + --> $DIR/floating_point_mul_add.rs:26:13 | LL | let _ = a + 2. * 4.; | ^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4., a)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:26:13 + --> $DIR/floating_point_mul_add.rs:28:13 | LL | let _ = (a * b) + c; | ^^^^^^^^^^^ help: consider using: `a.mul_add(b, c)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:27:13 + --> $DIR/floating_point_mul_add.rs:29:13 | LL | let _ = c + (a * b); | ^^^^^^^^^^^ help: consider using: `a.mul_add(b, c)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:28:13 + --> $DIR/floating_point_mul_add.rs:30:13 | LL | let _ = a * b * c + d; | ^^^^^^^^^^^^^ help: consider using: `(a * b).mul_add(c, d)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:30:13 + --> $DIR/floating_point_mul_add.rs:32:13 | LL | let _ = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c))` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:31:13 + --> $DIR/floating_point_mul_add.rs:33:13 | LL | let _ = 1234.567_f64 * 45.67834_f64 + 0.0004_f64; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `1234.567_f64.mul_add(45.67834_f64, 0.0004_f64)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_mul_add.rs:33:13 + --> $DIR/floating_point_mul_add.rs:35:13 | LL | let _ = (a * a + b).sqrt(); | ^^^^^^^^^^^ help: consider using: `a.mul_add(a, b)` -error: aborting due to 10 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed index 5758db7c6c8..cc337038888 100644 --- a/tests/ui/floating_point_powi.fixed +++ b/tests/ui/floating_point_powi.fixed @@ -7,7 +7,9 @@ fn main() { let y = 4f32; let _ = x.mul_add(x, y); + let _ = x.mul_add(x, -y); let _ = y.mul_add(y, x); + let _ = y.mul_add(-y, x); let _ = (y as f32).mul_add(y as f32, x); let _ = x.mul_add(x, y).sqrt(); let _ = y.mul_add(y, x).sqrt(); diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs index 5926bf1b000..5950902178c 100644 --- a/tests/ui/floating_point_powi.rs +++ b/tests/ui/floating_point_powi.rs @@ -7,7 +7,9 @@ fn main() { let y = 4f32; let _ = x.powi(2) + y; + let _ = x.powi(2) - y; let _ = x + y.powi(2); + let _ = x - y.powi(2); let _ = x + (y as f32).powi(2); let _ = (x.powi(2) + y).sqrt(); let _ = (x + y.powi(2)).sqrt(); diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr index a3c74544212..643b70739cd 100644 --- a/tests/ui/floating_point_powi.stderr +++ b/tests/ui/floating_point_powi.stderr @@ -9,26 +9,38 @@ LL | let _ = x.powi(2) + y; error: multiply and add expressions can be calculated more efficiently and accurately --> $DIR/floating_point_powi.rs:10:13 | +LL | let _ = x.powi(2) - y; + | ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, -y)` + +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:11:13 + | LL | let _ = x + y.powi(2); | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:11:13 + --> $DIR/floating_point_powi.rs:12:13 + | +LL | let _ = x - y.powi(2); + | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(-y, x)` + +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_powi.rs:13:13 | LL | let _ = x + (y as f32).powi(2); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(y as f32).mul_add(y as f32, x)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:12:13 + --> $DIR/floating_point_powi.rs:14:13 | LL | let _ = (x.powi(2) + y).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:13:13 + --> $DIR/floating_point_powi.rs:15:13 | LL | let _ = (x + y.powi(2)).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` -error: aborting due to 5 previous errors +error: aborting due to 7 previous errors