From a6f1af75d71fcf8e029b78142370e7563798c503 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Fri, 3 Apr 2020 13:58:52 -0300 Subject: [PATCH 1/9] Lint for x.powi(2) => x * x --- clippy_lints/src/floating_point_arithmetic.rs | 27 ++++++++++++++++++- tests/ui/floating_point_log.fixed | 10 +++---- tests/ui/floating_point_log.rs | 10 +++---- tests/ui/floating_point_log.stderr | 20 +++++++------- tests/ui/floating_point_powf.fixed | 4 +-- tests/ui/floating_point_powf.rs | 4 +-- tests/ui/floating_point_powf.stderr | 8 +++--- tests/ui/floating_point_powi.fixed | 12 +++++++++ tests/ui/floating_point_powi.rs | 12 +++++++++ tests/ui/floating_point_powi.stderr | 16 +++++++++++ 10 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 tests/ui/floating_point_powi.fixed create mode 100644 tests/ui/floating_point_powi.rs create mode 100644 tests/ui/floating_point_powi.stderr diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 4efd06892679..e3ee4296119d 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -1,6 +1,6 @@ use crate::consts::{ constant, constant_simple, Constant, - Constant::{F32, F64}, + Constant::{Int, F32, F64}, }; use crate::utils::{higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq}; use if_chain::if_chain; @@ -293,6 +293,30 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { } } +fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { + // Check argument + if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { + let (lint, help, suggestion) = match value { + Int(2) => ( + IMPRECISE_FLOPS, + "square can be computed more accurately", + format!("{} * {}", Sugg::hir(cx, &args[0], ".."), Sugg::hir(cx, &args[0], "..")), + ), + _ => return, + }; + + span_lint_and_sugg( + cx, + lint, + expr.span, + help, + "consider using", + suggestion, + Applicability::MachineApplicable, + ); + } +} + // TODO: Lint expressions of the form `x.exp() - y` where y > 1 // and suggest usage of `x.exp_m1() - (y - 1)` instead fn check_expm1(cx: &LateContext<'_>, expr: &Expr<'_>) { @@ -489,6 +513,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { "ln" => check_ln1p(cx, expr, args), "log" => check_log_base(cx, expr, args), "powf" => check_powf(cx, expr, args), + "powi" => check_powi(cx, expr, args), _ => {}, } } diff --git a/tests/ui/floating_point_log.fixed b/tests/ui/floating_point_log.fixed index 42c5e5d2bae2..7dc7ee94affc 100644 --- a/tests/ui/floating_point_log.fixed +++ b/tests/ui/floating_point_log.fixed @@ -25,11 +25,11 @@ fn check_ln1p() { let _ = 2.0f32.ln_1p(); let _ = x.ln_1p(); let _ = (x / 2.0).ln_1p(); - let _ = x.powi(2).ln_1p(); - let _ = (x.powi(2) / 2.0).ln_1p(); + let _ = x.powi(3).ln_1p(); + let _ = (x.powi(3) / 2.0).ln_1p(); let _ = ((std::f32::consts::E - 1.0)).ln_1p(); let _ = x.ln_1p(); - let _ = x.powi(2).ln_1p(); + let _ = x.powi(3).ln_1p(); let _ = (x + 2.0).ln_1p(); let _ = (x / 2.0).ln_1p(); // Cases where the lint shouldn't be applied @@ -43,9 +43,9 @@ fn check_ln1p() { let _ = 2.0f64.ln_1p(); let _ = x.ln_1p(); let _ = (x / 2.0).ln_1p(); - let _ = x.powi(2).ln_1p(); + let _ = x.powi(3).ln_1p(); let _ = x.ln_1p(); - let _ = x.powi(2).ln_1p(); + let _ = x.powi(3).ln_1p(); let _ = (x + 2.0).ln_1p(); let _ = (x / 2.0).ln_1p(); // Cases where the lint shouldn't be applied diff --git a/tests/ui/floating_point_log.rs b/tests/ui/floating_point_log.rs index 8be0d9ad56fc..01181484e7de 100644 --- a/tests/ui/floating_point_log.rs +++ b/tests/ui/floating_point_log.rs @@ -25,11 +25,11 @@ fn check_ln1p() { let _ = (1f32 + 2.0).ln(); let _ = (1.0 + x).ln(); let _ = (1.0 + x / 2.0).ln(); - let _ = (1.0 + x.powi(2)).ln(); - let _ = (1.0 + x.powi(2) / 2.0).ln(); + let _ = (1.0 + x.powi(3)).ln(); + let _ = (1.0 + x.powi(3) / 2.0).ln(); let _ = (1.0 + (std::f32::consts::E - 1.0)).ln(); let _ = (x + 1.0).ln(); - let _ = (x.powi(2) + 1.0).ln(); + let _ = (x.powi(3) + 1.0).ln(); let _ = (x + 2.0 + 1.0).ln(); let _ = (x / 2.0 + 1.0).ln(); // Cases where the lint shouldn't be applied @@ -43,9 +43,9 @@ fn check_ln1p() { let _ = (1f64 + 2.0).ln(); let _ = (1.0 + x).ln(); let _ = (1.0 + x / 2.0).ln(); - let _ = (1.0 + x.powi(2)).ln(); + let _ = (1.0 + x.powi(3)).ln(); let _ = (x + 1.0).ln(); - let _ = (x.powi(2) + 1.0).ln(); + let _ = (x.powi(3) + 1.0).ln(); let _ = (x + 2.0 + 1.0).ln(); let _ = (x / 2.0 + 1.0).ln(); // Cases where the lint shouldn't be applied diff --git a/tests/ui/floating_point_log.stderr b/tests/ui/floating_point_log.stderr index 943fbdb0b832..900dc2b79336 100644 --- a/tests/ui/floating_point_log.stderr +++ b/tests/ui/floating_point_log.stderr @@ -77,14 +77,14 @@ LL | let _ = (1.0 + x / 2.0).ln(); error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:28:13 | -LL | let _ = (1.0 + x.powi(2)).ln(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` +LL | let _ = (1.0 + x.powi(3)).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(3).ln_1p()` error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:29:13 | -LL | let _ = (1.0 + x.powi(2) / 2.0).ln(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x.powi(2) / 2.0).ln_1p()` +LL | let _ = (1.0 + x.powi(3) / 2.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x.powi(3) / 2.0).ln_1p()` error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:30:13 @@ -101,8 +101,8 @@ LL | let _ = (x + 1.0).ln(); error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:32:13 | -LL | let _ = (x.powi(2) + 1.0).ln(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` +LL | let _ = (x.powi(3) + 1.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(3).ln_1p()` error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:33:13 @@ -143,8 +143,8 @@ LL | let _ = (1.0 + x / 2.0).ln(); error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:46:13 | -LL | let _ = (1.0 + x.powi(2)).ln(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` +LL | let _ = (1.0 + x.powi(3)).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(3).ln_1p()` error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:47:13 @@ -155,8 +155,8 @@ LL | let _ = (x + 1.0).ln(); error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:48:13 | -LL | let _ = (x.powi(2) + 1.0).ln(); - | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` +LL | let _ = (x.powi(3) + 1.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(3).ln_1p()` error: ln(1 + x) can be computed more accurately --> $DIR/floating_point_log.rs:49:13 diff --git a/tests/ui/floating_point_powf.fixed b/tests/ui/floating_point_powf.fixed index 78a9d44829bb..b0641a100cdc 100644 --- a/tests/ui/floating_point_powf.fixed +++ b/tests/ui/floating_point_powf.fixed @@ -11,7 +11,7 @@ fn main() { let _ = (-3.1f32).exp(); let _ = x.sqrt(); let _ = x.cbrt(); - let _ = x.powi(2); + let _ = x.powi(3); let _ = x.powi(-2); let _ = x.powi(16_777_215); let _ = x.powi(-16_777_215); @@ -30,7 +30,7 @@ fn main() { let _ = (-3.1f64).exp(); let _ = x.sqrt(); let _ = x.cbrt(); - let _ = x.powi(2); + let _ = x.powi(3); let _ = x.powi(-2); let _ = x.powi(-2_147_483_648); let _ = x.powi(2_147_483_647); diff --git a/tests/ui/floating_point_powf.rs b/tests/ui/floating_point_powf.rs index dbc1cac5cb43..a0a2c973900f 100644 --- a/tests/ui/floating_point_powf.rs +++ b/tests/ui/floating_point_powf.rs @@ -11,7 +11,7 @@ fn main() { let _ = std::f32::consts::E.powf(-3.1); let _ = x.powf(1.0 / 2.0); let _ = x.powf(1.0 / 3.0); - let _ = x.powf(2.0); + let _ = x.powf(3.0); let _ = x.powf(-2.0); let _ = x.powf(16_777_215.0); let _ = x.powf(-16_777_215.0); @@ -30,7 +30,7 @@ fn main() { let _ = std::f64::consts::E.powf(-3.1); let _ = x.powf(1.0 / 2.0); let _ = x.powf(1.0 / 3.0); - let _ = x.powf(2.0); + let _ = x.powf(3.0); let _ = x.powf(-2.0); let _ = x.powf(-2_147_483_648.0); let _ = x.powf(2_147_483_647.0); diff --git a/tests/ui/floating_point_powf.stderr b/tests/ui/floating_point_powf.stderr index ad5163f0079b..2422eb911e90 100644 --- a/tests/ui/floating_point_powf.stderr +++ b/tests/ui/floating_point_powf.stderr @@ -53,8 +53,8 @@ LL | let _ = x.powf(1.0 / 3.0); error: exponentiation with integer powers can be computed more efficiently --> $DIR/floating_point_powf.rs:14:13 | -LL | let _ = x.powf(2.0); - | ^^^^^^^^^^^ help: consider using: `x.powi(2)` +LL | let _ = x.powf(3.0); + | ^^^^^^^^^^^ help: consider using: `x.powi(3)` error: exponentiation with integer powers can be computed more efficiently --> $DIR/floating_point_powf.rs:15:13 @@ -125,8 +125,8 @@ LL | let _ = x.powf(1.0 / 3.0); error: exponentiation with integer powers can be computed more efficiently --> $DIR/floating_point_powf.rs:33:13 | -LL | let _ = x.powf(2.0); - | ^^^^^^^^^^^ help: consider using: `x.powi(2)` +LL | let _ = x.powf(3.0); + | ^^^^^^^^^^^ help: consider using: `x.powi(3)` error: exponentiation with integer powers can be computed more efficiently --> $DIR/floating_point_powf.rs:34:13 diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed new file mode 100644 index 000000000000..0ce6f72535d1 --- /dev/null +++ b/tests/ui/floating_point_powi.fixed @@ -0,0 +1,12 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] + +fn main() { + let one = 1; + let x = 3f32; + let _ = x * x; + let _ = x * x; + // Cases where the lint shouldn't be applied + let _ = x.powi(3); + let _ = x.powi(one + 1); +} diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs new file mode 100644 index 000000000000..c87e836bedd9 --- /dev/null +++ b/tests/ui/floating_point_powi.rs @@ -0,0 +1,12 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] + +fn main() { + let one = 1; + let x = 3f32; + let _ = x.powi(2); + let _ = x.powi(1 + 1); + // Cases where the lint shouldn't be applied + let _ = x.powi(3); + let _ = x.powi(one + 1); +} diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr new file mode 100644 index 000000000000..ae7bbaa44738 --- /dev/null +++ b/tests/ui/floating_point_powi.stderr @@ -0,0 +1,16 @@ +error: square can be computed more accurately + --> $DIR/floating_point_powi.rs:7:13 + | +LL | let _ = x.powi(2); + | ^^^^^^^^^ help: consider using: `x * x` + | + = note: `-D clippy::imprecise-flops` implied by `-D warnings` + +error: square can be computed more accurately + --> $DIR/floating_point_powi.rs:8:13 + | +LL | let _ = x.powi(1 + 1); + | ^^^^^^^^^^^^^ help: consider using: `x * x` + +error: aborting due to 2 previous errors + From f62798454c8a7f9f2c3e87e0a913b3bd79b6d2ed Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 25 May 2020 13:54:39 -0300 Subject: [PATCH 2/9] Lint (x * x + y * y).sqrt() => x.hypot(y) --- clippy_lints/src/floating_point_arithmetic.rs | 75 ++++++++++++++++++- tests/ui/floating_point_hypot.fixed | 13 ++++ tests/ui/floating_point_hypot.rs | 13 ++++ tests/ui/floating_point_hypot.stderr | 30 ++++++++ tests/ui/floating_point_mul_add.fixed | 5 ++ tests/ui/floating_point_mul_add.rs | 5 ++ tests/ui/floating_point_mul_add.stderr | 8 +- tests/ui/floating_point_powi.fixed | 7 +- tests/ui/floating_point_powi.rs | 7 +- tests/ui/floating_point_powi.stderr | 20 ++++- 10 files changed, 177 insertions(+), 6 deletions(-) create mode 100644 tests/ui/floating_point_hypot.fixed create mode 100644 tests/ui/floating_point_hypot.rs create mode 100644 tests/ui/floating_point_hypot.stderr diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index e3ee4296119d..3b2e46a9a852 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -2,10 +2,10 @@ use crate::consts::{ constant, constant_simple, Constant, Constant::{Int, F32, F64}, }; -use crate::utils::{higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq}; +use crate::utils::{get_parent_expr, higher, numeric_literal, span_lint_and_sugg, sugg, SpanlessEq}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; +use rustc_hir::{BinOpKind, Expr, ExprKind, PathSegment, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -296,6 +296,17 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { // Check argument if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { + // TODO: need more specific check. this is too wide. remember also to include tests + if let Some(parent) = get_parent_expr(cx, expr) { + if let Some(grandparent) = get_parent_expr(cx, parent) { + if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = grandparent.kind { + if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { + return; + } + } + } + } + let (lint, help, suggestion) = match value { Int(2) => ( IMPRECISE_FLOPS, @@ -317,6 +328,57 @@ fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { } } +fn detect_hypot(cx: &LateContext<'_, '_>, args: &[Expr<'_>]) -> Option { + if let ExprKind::Binary( + Spanned { + node: BinOpKind::Add, .. + }, + ref add_lhs, + ref add_rhs, + ) = args[0].kind + { + // check if expression of the form x * x + y * y + if_chain! { + if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lmul_lhs, ref lmul_rhs) = add_lhs.kind; + if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref rmul_lhs, ref rmul_rhs) = add_rhs.kind; + if are_exprs_equal(cx, lmul_lhs, lmul_rhs); + if are_exprs_equal(cx, rmul_lhs, rmul_rhs); + then { + return Some(format!("{}.hypot({})", Sugg::hir(cx, &lmul_lhs, ".."), Sugg::hir(cx, &rmul_lhs, ".."))); + } + } + + // check if expression of the form x.powi(2) + y.powi(2) + if_chain! { + if let ExprKind::MethodCall(PathSegment { ident: lmethod_name, .. }, ref _lspan, ref largs) = add_lhs.kind; + if let ExprKind::MethodCall(PathSegment { ident: rmethod_name, .. }, ref _rspan, ref rargs) = add_rhs.kind; + if lmethod_name.as_str() == "powi" && rmethod_name.as_str() == "powi"; + if let Some((lvalue, _)) = constant(cx, cx.tables, &largs[1]); + if let Some((rvalue, _)) = constant(cx, cx.tables, &rargs[1]); + if Int(2) == lvalue && Int(2) == rvalue; + then { + return Some(format!("{}.hypot({})", Sugg::hir(cx, &largs[0], ".."), Sugg::hir(cx, &rargs[0], ".."))); + } + } + } + + None +} + +fn check_hypot(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { + if let Some(message) = detect_hypot(cx, args) { + span_lint_and_sugg( + cx, + IMPRECISE_FLOPS, + expr.span, + "hypotenuse can be computed more accurately", + "consider using", + message, + Applicability::MachineApplicable, + ); + } +} + // TODO: Lint expressions of the form `x.exp() - y` where y > 1 // and suggest usage of `x.exp_m1() - (y - 1)` instead fn check_expm1(cx: &LateContext<'_>, expr: &Expr<'_>) { @@ -368,6 +430,14 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { rhs, ) = &expr.kind { + if let Some(parent) = get_parent_expr(cx, expr) { + if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = parent.kind { + if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { + return; + } + } + } + let (recv, arg1, arg2) = if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, lhs) { (inner_lhs, inner_rhs, rhs) } else if let Some((inner_lhs, inner_rhs)) = is_float_mul_expr(cx, rhs) { @@ -514,6 +584,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { "log" => check_log_base(cx, expr, args), "powf" => check_powf(cx, expr, args), "powi" => check_powi(cx, expr, args), + "sqrt" => check_hypot(cx, expr, args), _ => {}, } } diff --git a/tests/ui/floating_point_hypot.fixed b/tests/ui/floating_point_hypot.fixed new file mode 100644 index 000000000000..f90695bc3fe7 --- /dev/null +++ b/tests/ui/floating_point_hypot.fixed @@ -0,0 +1,13 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] + +fn main() { + let x = 3f32; + let y = 4f32; + let _ = x.hypot(y); + let _ = (x + 1f32).hypot(y); + let _ = x.hypot(y); + // Cases where the lint shouldn't be applied + let _ = x.mul_add(x, y * y).sqrt(); + let _ = x.mul_add(4f32, y * y).sqrt(); +} diff --git a/tests/ui/floating_point_hypot.rs b/tests/ui/floating_point_hypot.rs new file mode 100644 index 000000000000..e7b048e262fa --- /dev/null +++ b/tests/ui/floating_point_hypot.rs @@ -0,0 +1,13 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] + +fn main() { + let x = 3f32; + let y = 4f32; + let _ = (x * x + y * y).sqrt(); + let _ = ((x + 1f32) * (x + 1f32) + y * y).sqrt(); + let _ = (x.powi(2) + y.powi(2)).sqrt(); + // Cases where the lint shouldn't be applied + let _ = x.mul_add(x, y * y).sqrt(); + let _ = (x * 4f32 + y * y).sqrt(); +} diff --git a/tests/ui/floating_point_hypot.stderr b/tests/ui/floating_point_hypot.stderr new file mode 100644 index 000000000000..fe1dfc7a4510 --- /dev/null +++ b/tests/ui/floating_point_hypot.stderr @@ -0,0 +1,30 @@ +error: hypotenuse can be computed more accurately + --> $DIR/floating_point_hypot.rs:7:13 + | +LL | let _ = (x * x + y * y).sqrt(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)` + | + = note: `-D clippy::imprecise-flops` implied by `-D warnings` + +error: hypotenuse can be computed more accurately + --> $DIR/floating_point_hypot.rs:8:13 + | +LL | let _ = ((x + 1f32) * (x + 1f32) + y * y).sqrt(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x + 1f32).hypot(y)` + +error: hypotenuse can be computed more accurately + --> $DIR/floating_point_hypot.rs:9:13 + | +LL | let _ = (x.powi(2) + y.powi(2)).sqrt(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)` + +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_hypot.rs:12:13 + | +LL | let _ = (x * 4f32 + y * y).sqrt(); + | ^^^^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(4f32, y * y)` + | + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` + +error: aborting due to 4 previous errors + diff --git a/tests/ui/floating_point_mul_add.fixed b/tests/ui/floating_point_mul_add.fixed index e343c37740da..911700bab004 100644 --- a/tests/ui/floating_point_mul_add.fixed +++ b/tests/ui/floating_point_mul_add.fixed @@ -18,4 +18,9 @@ fn main() { let _ = a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c)) + c; let _ = 1234.567_f64.mul_add(45.67834_f64, 0.0004_f64); + + let _ = a.mul_add(a, b).sqrt(); + + // Cases where the lint shouldn't be applied + let _ = (a * a + b * b).sqrt(); } diff --git a/tests/ui/floating_point_mul_add.rs b/tests/ui/floating_point_mul_add.rs index 810f929c8568..d202385fc8ae 100644 --- a/tests/ui/floating_point_mul_add.rs +++ b/tests/ui/floating_point_mul_add.rs @@ -18,4 +18,9 @@ fn main() { let _ = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c; let _ = 1234.567_f64 * 45.67834_f64 + 0.0004_f64; + + let _ = (a * a + b).sqrt(); + + // Cases where the lint shouldn't be applied + let _ = (a * a + b * b).sqrt(); } diff --git a/tests/ui/floating_point_mul_add.stderr b/tests/ui/floating_point_mul_add.stderr index 2dfbf562d15f..ac8d0c0cae06 100644 --- a/tests/ui/floating_point_mul_add.stderr +++ b/tests/ui/floating_point_mul_add.stderr @@ -54,5 +54,11 @@ error: multiply and add expressions can be calculated more efficiently and accur 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: aborting due to 9 previous errors +error: multiply and add expressions can be calculated more efficiently and accurately + --> $DIR/floating_point_mul_add.rs:22:13 + | +LL | let _ = (a * a + b).sqrt(); + | ^^^^^^^^^^^ help: consider using: `a.mul_add(a, b)` + +error: aborting due to 10 previous errors diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed index 0ce6f72535d1..98766e68aaf6 100644 --- a/tests/ui/floating_point_powi.fixed +++ b/tests/ui/floating_point_powi.fixed @@ -1,12 +1,17 @@ // run-rustfix -#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![warn(clippy::imprecise_flops)] fn main() { let one = 1; let x = 3f32; let _ = x * x; let _ = x * x; + + let y = 4f32; + let _ = (x * x + y).sqrt(); + let _ = (x + y * y).sqrt(); // Cases where the lint shouldn't be applied let _ = x.powi(3); let _ = x.powi(one + 1); + let _ = x.hypot(y); } diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs index c87e836bedd9..3c4b636a3d84 100644 --- a/tests/ui/floating_point_powi.rs +++ b/tests/ui/floating_point_powi.rs @@ -1,12 +1,17 @@ // run-rustfix -#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![warn(clippy::imprecise_flops)] fn main() { let one = 1; let x = 3f32; let _ = x.powi(2); let _ = x.powi(1 + 1); + + let y = 4f32; + let _ = (x.powi(2) + y).sqrt(); + let _ = (x + y.powi(2)).sqrt(); // Cases where the lint shouldn't be applied let _ = x.powi(3); let _ = x.powi(one + 1); + let _ = (x.powi(2) + y.powi(2)).sqrt(); } diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr index ae7bbaa44738..f370e24bf052 100644 --- a/tests/ui/floating_point_powi.stderr +++ b/tests/ui/floating_point_powi.stderr @@ -12,5 +12,23 @@ error: square can be computed more accurately LL | let _ = x.powi(1 + 1); | ^^^^^^^^^^^^^ help: consider using: `x * x` -error: aborting due to 2 previous errors +error: square can be computed more accurately + --> $DIR/floating_point_powi.rs:11:14 + | +LL | let _ = (x.powi(2) + y).sqrt(); + | ^^^^^^^^^ help: consider using: `x * x` + +error: square can be computed more accurately + --> $DIR/floating_point_powi.rs:12:18 + | +LL | let _ = (x + y.powi(2)).sqrt(); + | ^^^^^^^^^ help: consider using: `y * y` + +error: hypotenuse can be computed more accurately + --> $DIR/floating_point_powi.rs:16:13 + | +LL | let _ = (x.powi(2) + y.powi(2)).sqrt(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)` + +error: aborting due to 5 previous errors From 0c8afa39ce8b2e7f0f9be7fc33fe3993d9bc3c53 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 1 Jun 2020 18:32:52 -0300 Subject: [PATCH 3/9] Lint x.log(b) / y.log(b) => x.log(y) --- clippy_lints/src/floating_point_arithmetic.rs | 65 ++++++++++++++++--- tests/ui/floating_point_logbase.fixed | 16 +++++ tests/ui/floating_point_logbase.rs | 16 +++++ tests/ui/floating_point_logbase.stderr | 28 ++++++++ 4 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 tests/ui/floating_point_logbase.fixed create mode 100644 tests/ui/floating_point_logbase.rs create mode 100644 tests/ui/floating_point_logbase.stderr diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 3b2e46a9a852..9f241c2c3a2b 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -293,13 +293,13 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { } } -fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { +fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { // Check argument - if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { + if let Some((value, _)) = constant(cx, cx.tables(), &args[1]) { // TODO: need more specific check. this is too wide. remember also to include tests if let Some(parent) = get_parent_expr(cx, expr) { if let Some(grandparent) = get_parent_expr(cx, parent) { - if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = grandparent.kind { + if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args, _) = grandparent.kind { if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { return; } @@ -328,7 +328,7 @@ fn check_powi(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { } } -fn detect_hypot(cx: &LateContext<'_, '_>, args: &[Expr<'_>]) -> Option { +fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option { if let ExprKind::Binary( Spanned { node: BinOpKind::Add, .. @@ -350,11 +350,11 @@ fn detect_hypot(cx: &LateContext<'_, '_>, args: &[Expr<'_>]) -> Option { // check if expression of the form x.powi(2) + y.powi(2) if_chain! { - if let ExprKind::MethodCall(PathSegment { ident: lmethod_name, .. }, ref _lspan, ref largs) = add_lhs.kind; - if let ExprKind::MethodCall(PathSegment { ident: rmethod_name, .. }, ref _rspan, ref rargs) = add_rhs.kind; + if let ExprKind::MethodCall(PathSegment { ident: lmethod_name, .. }, ref _lspan, ref largs, _) = add_lhs.kind; + if let ExprKind::MethodCall(PathSegment { ident: rmethod_name, .. }, ref _rspan, ref rargs, _) = add_rhs.kind; if lmethod_name.as_str() == "powi" && rmethod_name.as_str() == "powi"; - if let Some((lvalue, _)) = constant(cx, cx.tables, &largs[1]); - if let Some((rvalue, _)) = constant(cx, cx.tables, &rargs[1]); + if let Some((lvalue, _)) = constant(cx, cx.tables(), &largs[1]); + if let Some((rvalue, _)) = constant(cx, cx.tables(), &rargs[1]); if Int(2) == lvalue && Int(2) == rvalue; then { return Some(format!("{}.hypot({})", Sugg::hir(cx, &largs[0], ".."), Sugg::hir(cx, &rargs[0], ".."))); @@ -365,7 +365,7 @@ fn detect_hypot(cx: &LateContext<'_, '_>, args: &[Expr<'_>]) -> Option { None } -fn check_hypot(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) { +fn check_hypot(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { if let Some(message) = detect_hypot(cx, args) { span_lint_and_sugg( cx, @@ -431,7 +431,7 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { ) = &expr.kind { if let Some(parent) = get_parent_expr(cx, expr) { - if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args) = parent.kind { + if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args, _) = parent.kind { if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { return; } @@ -573,6 +573,50 @@ fn check_custom_abs(cx: &LateContext<'_>, expr: &Expr<'_>) { } } +fn are_same_base_logs(cx: &LateContext<'_>, expr_a: &Expr<'_>, expr_b: &Expr<'_>) -> bool { + if_chain! { + if let ExprKind::MethodCall(PathSegment { ident: method_name_a, .. }, _, ref args_a, _) = expr_a.kind; + if let ExprKind::MethodCall(PathSegment { ident: method_name_b, .. }, _, ref args_b, _) = expr_b.kind; + then { + return method_name_a.as_str() == method_name_b.as_str() && + args_a.len() == args_b.len() && + ( + ["ln", "log2", "log10"].contains(&&*method_name_a.as_str()) || + method_name_a.as_str() == "log" && args_a.len() == 2 && are_exprs_equal(cx, &args_a[1], &args_b[1]) + ); + } + } + + false +} + +fn check_log_division(cx: &LateContext<'_>, expr: &Expr<'_>) { + // check if expression of the form x.logN() / y.logN() + if_chain! { + if let ExprKind::Binary( + Spanned { + node: BinOpKind::Div, .. + }, + lhs, + rhs, + ) = &expr.kind; + if are_same_base_logs(cx, lhs, rhs); + if let ExprKind::MethodCall(_, _, ref largs, _) = lhs.kind; + if let ExprKind::MethodCall(_, _, ref rargs, _) = rhs.kind; + then { + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "division of logarithms can be calculated more efficiently and accurately", + "consider using", + format!("{}.log({})", Sugg::hir(cx, &largs[0], ".."), Sugg::hir(cx, &rargs[0], ".."),), + Applicability::MachineApplicable, + ); + } + } +} + impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let ExprKind::MethodCall(ref path, _, args, _) = &expr.kind { @@ -592,6 +636,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { check_expm1(cx, expr); check_mul_add(cx, expr); check_custom_abs(cx, expr); + check_log_division(cx, expr); } } } diff --git a/tests/ui/floating_point_logbase.fixed b/tests/ui/floating_point_logbase.fixed new file mode 100644 index 000000000000..13962a272d45 --- /dev/null +++ b/tests/ui/floating_point_logbase.fixed @@ -0,0 +1,16 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops)] + +fn main() { + let x = 3f32; + let y = 5f32; + let _ = x.log(y); + let _ = x.log(y); + let _ = x.log(y); + let _ = x.log(y); + // Cases where the lint shouldn't be applied + let _ = x.ln() / y.powf(3.2); + let _ = x.powf(3.2) / y.powf(3.2); + let _ = x.powf(3.2) / y.ln(); + let _ = x.log(5f32) / y.log(7f32); +} diff --git a/tests/ui/floating_point_logbase.rs b/tests/ui/floating_point_logbase.rs new file mode 100644 index 000000000000..26bc20d5370b --- /dev/null +++ b/tests/ui/floating_point_logbase.rs @@ -0,0 +1,16 @@ +// run-rustfix +#![warn(clippy::suboptimal_flops)] + +fn main() { + let x = 3f32; + let y = 5f32; + let _ = x.ln() / y.ln(); + let _ = x.log2() / y.log2(); + let _ = x.log10() / y.log10(); + let _ = x.log(5f32) / y.log(5f32); + // Cases where the lint shouldn't be applied + let _ = x.ln() / y.powf(3.2); + let _ = x.powf(3.2) / y.powf(3.2); + let _ = x.powf(3.2) / y.ln(); + let _ = x.log(5f32) / y.log(7f32); +} diff --git a/tests/ui/floating_point_logbase.stderr b/tests/ui/floating_point_logbase.stderr new file mode 100644 index 000000000000..fa956b9139eb --- /dev/null +++ b/tests/ui/floating_point_logbase.stderr @@ -0,0 +1,28 @@ +error: division of logarithms can be calculated more efficiently and accurately + --> $DIR/floating_point_logbase.rs:7:13 + | +LL | let _ = x.ln() / y.ln(); + | ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + | + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` + +error: division of logarithms can be calculated more efficiently and accurately + --> $DIR/floating_point_logbase.rs:8:13 + | +LL | let _ = x.log2() / y.log2(); + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: division of logarithms can be calculated more efficiently and accurately + --> $DIR/floating_point_logbase.rs:9:13 + | +LL | let _ = x.log10() / y.log10(); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: division of logarithms can be calculated more efficiently and accurately + --> $DIR/floating_point_logbase.rs:10:13 + | +LL | let _ = x.log(5f32) / y.log(5f32); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: aborting due to 4 previous errors + From 076ec872ce122403f3c75f20c773c64b194a5891 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Wed, 10 Jun 2020 13:52:00 -0300 Subject: [PATCH 4/9] Lint for to_radians and to_degrees --- clippy_lints/src/floating_point_arithmetic.rs | 64 ++++++++++++++++++- tests/ui/floating_point_rad.fixed | 13 ++++ tests/ui/floating_point_rad.rs | 13 ++++ tests/ui/floating_point_rad.stderr | 16 +++++ 4 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 tests/ui/floating_point_rad.fixed create mode 100644 tests/ui/floating_point_rad.rs create mode 100644 tests/ui/floating_point_rad.stderr diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 9f241c2c3a2b..d88e47f396cf 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -350,8 +350,18 @@ fn detect_hypot(cx: &LateContext<'_>, args: &[Expr<'_>]) -> Option { // check if expression of the form x.powi(2) + y.powi(2) if_chain! { - if let ExprKind::MethodCall(PathSegment { ident: lmethod_name, .. }, ref _lspan, ref largs, _) = add_lhs.kind; - if let ExprKind::MethodCall(PathSegment { ident: rmethod_name, .. }, ref _rspan, ref rargs, _) = add_rhs.kind; + if let ExprKind::MethodCall( + PathSegment { ident: lmethod_name, .. }, + ref _lspan, + ref largs, + _ + ) = add_lhs.kind; + if let ExprKind::MethodCall( + PathSegment { ident: rmethod_name, .. }, + ref _rspan, + ref rargs, + _ + ) = add_rhs.kind; if lmethod_name.as_str() == "powi" && rmethod_name.as_str() == "powi"; if let Some((lvalue, _)) = constant(cx, cx.tables(), &largs[1]); if let Some((rvalue, _)) = constant(cx, cx.tables(), &rargs[1]); @@ -617,6 +627,55 @@ fn check_log_division(cx: &LateContext<'_>, expr: &Expr<'_>) { } } +fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + if let ExprKind::Binary( + Spanned { + node: BinOpKind::Div, .. + }, + div_lhs, + div_rhs, + ) = &expr.kind; + if let ExprKind::Binary( + Spanned { + node: BinOpKind::Mul, .. + }, + mul_lhs, + mul_rhs, + ) = &div_lhs.kind; + if let Some((rvalue, _)) = constant(cx, cx.tables(), div_rhs); + if let Some((lvalue, _)) = constant(cx, cx.tables(), mul_rhs); + then { + if (F32(f32_consts::PI) == rvalue || F64(f64_consts::PI) == rvalue) && + (F32(180_f32) == lvalue || F64(180_f64) == lvalue) + { + span_lint_and_sugg( + cx, + IMPRECISE_FLOPS, + expr.span, + "conversion to degrees can be done more accurately", + "consider using", + format!("{}.to_degrees()", Sugg::hir(cx, &mul_lhs, "..")), + Applicability::MachineApplicable, + ); + } else if + (F32(180_f32) == rvalue || F64(180_f64) == rvalue) && + (F32(f32_consts::PI) == lvalue || F64(f64_consts::PI) == lvalue) + { + span_lint_and_sugg( + cx, + IMPRECISE_FLOPS, + expr.span, + "conversion to radians can be done more accurately", + "consider using", + format!("{}.to_radians()", Sugg::hir(cx, &mul_lhs, "..")), + Applicability::MachineApplicable, + ); + } + } + } +} + impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if let ExprKind::MethodCall(ref path, _, args, _) = &expr.kind { @@ -637,6 +696,7 @@ impl<'tcx> LateLintPass<'tcx> for FloatingPointArithmetic { check_mul_add(cx, expr); check_custom_abs(cx, expr); check_log_division(cx, expr); + check_radians(cx, expr); } } } diff --git a/tests/ui/floating_point_rad.fixed b/tests/ui/floating_point_rad.fixed new file mode 100644 index 000000000000..64461417a6a1 --- /dev/null +++ b/tests/ui/floating_point_rad.fixed @@ -0,0 +1,13 @@ +// run-rustfix +#![warn(clippy::imprecise_flops)] + +fn main() { + let x = 3f32; + let _ = x.to_degrees(); + let _ = x.to_radians(); + // Cases where the lint shouldn't be applied + let _ = x * 90f32 / std::f32::consts::PI; + let _ = x * std::f32::consts::PI / 90f32; + let _ = x * 180f32 / std::f32::consts::E; + let _ = x * std::f32::consts::E / 180f32; +} diff --git a/tests/ui/floating_point_rad.rs b/tests/ui/floating_point_rad.rs new file mode 100644 index 000000000000..9046f184b3e5 --- /dev/null +++ b/tests/ui/floating_point_rad.rs @@ -0,0 +1,13 @@ +// run-rustfix +#![warn(clippy::imprecise_flops)] + +fn main() { + let x = 3f32; + let _ = x * 180f32 / std::f32::consts::PI; + let _ = x * std::f32::consts::PI / 180f32; + // Cases where the lint shouldn't be applied + let _ = x * 90f32 / std::f32::consts::PI; + let _ = x * std::f32::consts::PI / 90f32; + let _ = x * 180f32 / std::f32::consts::E; + let _ = x * std::f32::consts::E / 180f32; +} diff --git a/tests/ui/floating_point_rad.stderr b/tests/ui/floating_point_rad.stderr new file mode 100644 index 000000000000..81e818215137 --- /dev/null +++ b/tests/ui/floating_point_rad.stderr @@ -0,0 +1,16 @@ +error: conversion to degrees can be done more accurately + --> $DIR/floating_point_rad.rs:6:13 + | +LL | let _ = x * 180f32 / std::f32::consts::PI; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.to_degrees()` + | + = note: `-D clippy::imprecise-flops` implied by `-D warnings` + +error: conversion to radians can be done more accurately + --> $DIR/floating_point_rad.rs:7:13 + | +LL | let _ = x * std::f32::consts::PI / 180f32; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.to_radians()` + +error: aborting due to 2 previous errors + From f5596826fa59035e6c57d8968df0d61f69c2537b Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 15 Jun 2020 13:55:12 -0300 Subject: [PATCH 5/9] Better copy for lint message Since x.log(y) is actually implemented as x.ln() / y.ln() --- clippy_lints/src/floating_point_arithmetic.rs | 2 +- tests/ui/floating_point_logbase.stderr | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index d88e47f396cf..b1e258f4b160 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -618,7 +618,7 @@ fn check_log_division(cx: &LateContext<'_>, expr: &Expr<'_>) { cx, SUBOPTIMAL_FLOPS, expr.span, - "division of logarithms can be calculated more efficiently and accurately", + "log base can be expressed more clearly", "consider using", format!("{}.log({})", Sugg::hir(cx, &largs[0], ".."), Sugg::hir(cx, &rargs[0], ".."),), Applicability::MachineApplicable, diff --git a/tests/ui/floating_point_logbase.stderr b/tests/ui/floating_point_logbase.stderr index fa956b9139eb..78354c2f62d4 100644 --- a/tests/ui/floating_point_logbase.stderr +++ b/tests/ui/floating_point_logbase.stderr @@ -1,4 +1,4 @@ -error: division of logarithms can be calculated more efficiently and accurately +error: log base can be expressed more clearly --> $DIR/floating_point_logbase.rs:7:13 | LL | let _ = x.ln() / y.ln(); @@ -6,19 +6,19 @@ LL | let _ = x.ln() / y.ln(); | = note: `-D clippy::suboptimal-flops` implied by `-D warnings` -error: division of logarithms can be calculated more efficiently and accurately +error: log base can be expressed more clearly --> $DIR/floating_point_logbase.rs:8:13 | LL | let _ = x.log2() / y.log2(); | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` -error: division of logarithms can be calculated more efficiently and accurately +error: log base can be expressed more clearly --> $DIR/floating_point_logbase.rs:9:13 | LL | let _ = x.log10() / y.log10(); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` -error: division of logarithms can be calculated more efficiently and accurately +error: log base can be expressed more clearly --> $DIR/floating_point_logbase.rs:10:13 | LL | let _ = x.log(5f32) / y.log(5f32); From db7bc6b3bd0e58c8fbf7507713a4214299f39c36 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 15 Jun 2020 13:59:44 -0300 Subject: [PATCH 6/9] Place radian lints under suboptimal_flops --- clippy_lints/src/floating_point_arithmetic.rs | 4 ++-- tests/ui/floating_point_rad.fixed | 2 +- tests/ui/floating_point_rad.rs | 2 +- tests/ui/floating_point_rad.stderr | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index b1e258f4b160..beb0b234408b 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -651,7 +651,7 @@ fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { { span_lint_and_sugg( cx, - IMPRECISE_FLOPS, + SUBOPTIMAL_FLOPS, expr.span, "conversion to degrees can be done more accurately", "consider using", @@ -664,7 +664,7 @@ fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { { span_lint_and_sugg( cx, - IMPRECISE_FLOPS, + SUBOPTIMAL_FLOPS, expr.span, "conversion to radians can be done more accurately", "consider using", diff --git a/tests/ui/floating_point_rad.fixed b/tests/ui/floating_point_rad.fixed index 64461417a6a1..92480c5db8be 100644 --- a/tests/ui/floating_point_rad.fixed +++ b/tests/ui/floating_point_rad.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::imprecise_flops)] +#![warn(clippy::suboptimal_flops)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_rad.rs b/tests/ui/floating_point_rad.rs index 9046f184b3e5..062e7c3fdc17 100644 --- a/tests/ui/floating_point_rad.rs +++ b/tests/ui/floating_point_rad.rs @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::imprecise_flops)] +#![warn(clippy::suboptimal_flops)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_rad.stderr b/tests/ui/floating_point_rad.stderr index 81e818215137..a6ffdca64eef 100644 --- a/tests/ui/floating_point_rad.stderr +++ b/tests/ui/floating_point_rad.stderr @@ -4,7 +4,7 @@ error: conversion to degrees can be done more accurately LL | let _ = x * 180f32 / std::f32::consts::PI; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.to_degrees()` | - = note: `-D clippy::imprecise-flops` implied by `-D warnings` + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` error: conversion to radians can be done more accurately --> $DIR/floating_point_rad.rs:7:13 From 6dc066fdb9103122cd918b1b38e26fa6edbc7e2e Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Wed, 17 Jun 2020 13:43:11 -0300 Subject: [PATCH 7/9] Includes TODO comment for hypot lint --- tests/ui/floating_point_hypot.fixed | 5 +++-- tests/ui/floating_point_hypot.rs | 3 ++- tests/ui/floating_point_hypot.stderr | 10 +--------- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/ui/floating_point_hypot.fixed b/tests/ui/floating_point_hypot.fixed index f90695bc3fe7..bbe411b3f488 100644 --- a/tests/ui/floating_point_hypot.fixed +++ b/tests/ui/floating_point_hypot.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![warn(clippy::imprecise_flops)] fn main() { let x = 3f32; @@ -8,6 +8,7 @@ fn main() { let _ = (x + 1f32).hypot(y); let _ = x.hypot(y); // Cases where the lint shouldn't be applied + // TODO: linting this adds some complexity, but could be done let _ = x.mul_add(x, y * y).sqrt(); - let _ = x.mul_add(4f32, y * y).sqrt(); + let _ = (x * 4f32 + y * y).sqrt(); } diff --git a/tests/ui/floating_point_hypot.rs b/tests/ui/floating_point_hypot.rs index e7b048e262fa..586fd170ea14 100644 --- a/tests/ui/floating_point_hypot.rs +++ b/tests/ui/floating_point_hypot.rs @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![warn(clippy::imprecise_flops)] fn main() { let x = 3f32; @@ -8,6 +8,7 @@ fn main() { let _ = ((x + 1f32) * (x + 1f32) + y * y).sqrt(); let _ = (x.powi(2) + y.powi(2)).sqrt(); // Cases where the lint shouldn't be applied + // TODO: linting this adds some complexity, but could be done let _ = x.mul_add(x, y * y).sqrt(); let _ = (x * 4f32 + y * y).sqrt(); } diff --git a/tests/ui/floating_point_hypot.stderr b/tests/ui/floating_point_hypot.stderr index fe1dfc7a4510..42069d9ee9ef 100644 --- a/tests/ui/floating_point_hypot.stderr +++ b/tests/ui/floating_point_hypot.stderr @@ -18,13 +18,5 @@ error: hypotenuse can be computed more accurately LL | let _ = (x.powi(2) + y.powi(2)).sqrt(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)` -error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_hypot.rs:12:13 - | -LL | let _ = (x * 4f32 + y * y).sqrt(); - | ^^^^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(4f32, y * y)` - | - = note: `-D clippy::suboptimal-flops` implied by `-D warnings` - -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors From 6be9491eace540d341f3b8dbf775a10e25f6431a Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 22 Jun 2020 14:16:27 -0300 Subject: [PATCH 8/9] Reclassify powi(2) lint under suboptimal_flops --- clippy_lints/src/floating_point_arithmetic.rs | 69 ++++++++++++------- tests/ui/floating_point_powi.fixed | 10 +-- tests/ui/floating_point_powi.rs | 4 +- tests/ui/floating_point_powi.stderr | 38 +++++----- 4 files changed, 75 insertions(+), 46 deletions(-) diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index beb0b234408b..11bd0ae23aa3 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -294,37 +294,56 @@ fn check_powf(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { } fn check_powi(cx: &LateContext<'_>, expr: &Expr<'_>, args: &[Expr<'_>]) { - // Check argument if let Some((value, _)) = constant(cx, cx.tables(), &args[1]) { - // TODO: need more specific check. this is too wide. remember also to include tests - if let Some(parent) = get_parent_expr(cx, expr) { - if let Some(grandparent) = get_parent_expr(cx, parent) { - if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args, _) = grandparent.kind { - if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { - return; + if value == Int(2) { + if let Some(parent) = get_parent_expr(cx, expr) { + if let Some(grandparent) = get_parent_expr(cx, parent) { + if let ExprKind::MethodCall(PathSegment { ident: method_name, .. }, _, args, _) = grandparent.kind { + if method_name.as_str() == "sqrt" && detect_hypot(cx, args).is_some() { + return; + } } } + + if let ExprKind::Binary( + Spanned { + node: BinOpKind::Add, .. + }, + ref lhs, + ref rhs, + ) = parent.kind + { + let other_addend = if lhs.hir_id == expr.hir_id { rhs } else { lhs }; + + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + parent.span, + "square can be computed more efficiently", + "consider using", + format!( + "{}.mul_add({}, {})", + Sugg::hir(cx, &args[0], ".."), + Sugg::hir(cx, &args[0], ".."), + Sugg::hir(cx, &other_addend, ".."), + ), + Applicability::MachineApplicable, + ); + + return; + } } - } - let (lint, help, suggestion) = match value { - Int(2) => ( - IMPRECISE_FLOPS, - "square can be computed more accurately", + span_lint_and_sugg( + cx, + SUBOPTIMAL_FLOPS, + expr.span, + "square can be computed more efficiently", + "consider using", format!("{} * {}", Sugg::hir(cx, &args[0], ".."), Sugg::hir(cx, &args[0], "..")), - ), - _ => return, - }; - - span_lint_and_sugg( - cx, - lint, - expr.span, - help, - "consider using", - suggestion, - Applicability::MachineApplicable, - ); + Applicability::MachineApplicable, + ); + } } } diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed index 98766e68aaf6..56762400593b 100644 --- a/tests/ui/floating_point_powi.fixed +++ b/tests/ui/floating_point_powi.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::imprecise_flops)] +#![warn(clippy::suboptimal_flops)] fn main() { let one = 1; @@ -8,10 +8,12 @@ fn main() { let _ = x * x; let y = 4f32; - let _ = (x * x + y).sqrt(); - let _ = (x + y * y).sqrt(); + let _ = x.mul_add(x, y); + let _ = y.mul_add(y, x); + let _ = x.mul_add(x, y).sqrt(); + let _ = y.mul_add(y, x).sqrt(); // Cases where the lint shouldn't be applied let _ = x.powi(3); let _ = x.powi(one + 1); - let _ = x.hypot(y); + let _ = (x.powi(2) + y.powi(2)).sqrt(); } diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs index 3c4b636a3d84..1f800e4628dc 100644 --- a/tests/ui/floating_point_powi.rs +++ b/tests/ui/floating_point_powi.rs @@ -1,5 +1,5 @@ // run-rustfix -#![warn(clippy::imprecise_flops)] +#![warn(clippy::suboptimal_flops)] fn main() { let one = 1; @@ -8,6 +8,8 @@ fn main() { let _ = x.powi(1 + 1); let y = 4f32; + let _ = x.powi(2) + y; + let _ = x + y.powi(2); let _ = (x.powi(2) + y).sqrt(); let _ = (x + y.powi(2)).sqrt(); // Cases where the lint shouldn't be applied diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr index f370e24bf052..d5a5f1bcca10 100644 --- a/tests/ui/floating_point_powi.stderr +++ b/tests/ui/floating_point_powi.stderr @@ -1,34 +1,40 @@ -error: square can be computed more accurately +error: square can be computed more efficiently --> $DIR/floating_point_powi.rs:7:13 | LL | let _ = x.powi(2); | ^^^^^^^^^ help: consider using: `x * x` | - = note: `-D clippy::imprecise-flops` implied by `-D warnings` + = note: `-D clippy::suboptimal-flops` implied by `-D warnings` -error: square can be computed more accurately +error: square can be computed more efficiently --> $DIR/floating_point_powi.rs:8:13 | LL | let _ = x.powi(1 + 1); | ^^^^^^^^^^^^^ help: consider using: `x * x` -error: square can be computed more accurately - --> $DIR/floating_point_powi.rs:11:14 +error: square can be computed more efficiently + --> $DIR/floating_point_powi.rs:11:13 | -LL | let _ = (x.powi(2) + y).sqrt(); - | ^^^^^^^^^ help: consider using: `x * x` +LL | let _ = x.powi(2) + y; + | ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` -error: square can be computed more accurately - --> $DIR/floating_point_powi.rs:12:18 +error: square can be computed more efficiently + --> $DIR/floating_point_powi.rs:12:13 | -LL | let _ = (x + y.powi(2)).sqrt(); - | ^^^^^^^^^ help: consider using: `y * y` +LL | let _ = x + y.powi(2); + | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` + +error: square can be computed more efficiently + --> $DIR/floating_point_powi.rs:13:13 + | +LL | let _ = (x.powi(2) + y).sqrt(); + | ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` -error: hypotenuse can be computed more accurately - --> $DIR/floating_point_powi.rs:16:13 +error: square can be computed more efficiently + --> $DIR/floating_point_powi.rs:14:13 | -LL | let _ = (x.powi(2) + y.powi(2)).sqrt(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.hypot(y)` +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 6 previous errors From 3065201eb3a0c4976de7ef5b5b924afde1b18325 Mon Sep 17 00:00:00 2001 From: Thiago Arrais Date: Mon, 6 Jul 2020 13:14:52 -0300 Subject: [PATCH 9/9] =?UTF-8?q?Includes=20TODO=20for=20constants=20equival?= =?UTF-8?q?ent=20to=20=CF=80/180?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- clippy_lints/src/floating_point_arithmetic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 11bd0ae23aa3..3087d6a940a8 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -665,6 +665,7 @@ fn check_radians(cx: &LateContext<'_>, expr: &Expr<'_>) { if let Some((rvalue, _)) = constant(cx, cx.tables(), div_rhs); if let Some((lvalue, _)) = constant(cx, cx.tables(), mul_rhs); then { + // TODO: also check for constant values near PI/180 or 180/PI if (F32(f32_consts::PI) == rvalue || F64(f64_consts::PI) == rvalue) && (F32(180_f32) == lvalue || F64(180_f64) == lvalue) {