diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index 84a9cac5b634..7f6dac87d04a 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -64,29 +64,31 @@ declare_clippy_lint! { declare_lint_pass!(FloatingPointArithmetic => [FLOATING_POINT_IMPROVEMENTS]); -fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec) { - let arg = sugg::Sugg::hir(cx, &args[0], "..").maybe_par(); - - if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { - let method; - +// Returns the specialized log method for a given base if base is constant +// and is one of 2, 10 and e +fn get_specialized_log_method(cx: &LateContext<'_, '_>, base: &Expr) -> Option<&'static str> { + if let Some((value, _)) = constant(cx, cx.tables, base) { if F32(2.0) == value || F64(2.0) == value { - method = "log2"; + return Some("log2"); } else if F32(10.0) == value || F64(10.0) == value { - method = "log10"; + return Some("log10"); } else if F32(f32_consts::E) == value || F64(f64_consts::E) == value { - method = "ln"; - } else { - return; + return Some("ln"); } + } + None +} + +fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec) { + if let Some(method) = get_specialized_log_method(cx, &args[1]) { span_lint_and_sugg( cx, FLOATING_POINT_IMPROVEMENTS, expr.span, "logarithm for bases 2, 10 and e can be computed more accurately", "consider using", - format!("{}.{}()", arg, method), + format!("{}.{}()", sugg::Sugg::hir(cx, &args[0], ".."), method), Applicability::MachineApplicable, ); } @@ -232,6 +234,82 @@ fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr) { } } +// Checks whether two expressions evaluate to the same value +fn are_exprs_equivalent(cx: &LateContext<'_, '_>, left: &Expr, right: &Expr) -> bool { + // Checks whether the values are constant and equal + if_chain! { + if let Some((left_value, _)) = constant(cx, cx.tables, left); + if let Some((right_value, _)) = constant(cx, cx.tables, right); + if left_value == right_value; + then { + return true; + } + } + + // Checks whether the expressions resolve to the same variable + if_chain! { + if let ExprKind::Path(ref left_qpath) = left.kind; + if let QPath::Resolved(_, ref left_path) = *left_qpath; + if left_path.segments.len() == 1; + if let def::Res::Local(left_local_id) = qpath_res(cx, left_qpath, left.hir_id); + if let ExprKind::Path(ref right_qpath) = right.kind; + if let QPath::Resolved(_, ref right_path) = *right_qpath; + if right_path.segments.len() == 1; + if let def::Res::Local(right_local_id) = qpath_res(cx, right_qpath, right.hir_id); + if left_local_id == right_local_id; + then { + return true; + } + } + + false +} + +fn check_log_division(cx: &LateContext<'_, '_>, expr: &Expr) { + let log_methods = ["log", "log2", "log10", "ln"]; + + if_chain! { + if let ExprKind::Binary(op, ref lhs, ref rhs) = expr.kind; + if op.node == BinOpKind::Div; + if cx.tables.expr_ty(lhs).is_floating_point(); + if let ExprKind::MethodCall(left_path, _, left_args) = &lhs.kind; + if let ExprKind::MethodCall(right_path, _, right_args) = &rhs.kind; + let left_method = left_path.ident.name.as_str(); + if left_method == right_path.ident.name.as_str(); + if log_methods.iter().any(|&method| left_method == method); + then { + let left_recv = &left_args[0]; + let right_recv = &right_args[0]; + + // Return early when bases are not equal + if left_method == "log" && !are_exprs_equivalent(cx, &left_args[1], &right_args[1]) { + return; + } + + // Reduce the expression further for bases 2, 10 and e + let suggestion = if let Some(method) = get_specialized_log_method(cx, right_recv) { + format!("{}.{}()", sugg::Sugg::hir(cx, left_recv, ".."), method) + } else { + format!( + "{}.log({})", + sugg::Sugg::hir(cx, left_recv, ".."), + sugg::Sugg::hir(cx, right_recv, "..") + ) + }; + + span_lint_and_sugg( + cx, + FLOATING_POINT_IMPROVEMENTS, + expr.span, + "x.log(b) / y.log(b) can be reduced to x.log(y)", + "consider using", + suggestion, + Applicability::MachineApplicable, + ); + } + } +} + impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { if let ExprKind::MethodCall(ref path, _, args) = &expr.kind { @@ -247,6 +325,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic { } } else { check_expm1(cx, expr); + check_log_division(cx, expr); } } } diff --git a/tests/ui/floating_point_arithmetic.rs b/tests/ui/floating_point_arithmetic.rs index 77a14a7a6668..ff5949415461 100644 --- a/tests/ui/floating_point_arithmetic.rs +++ b/tests/ui/floating_point_arithmetic.rs @@ -82,4 +82,30 @@ fn check_expm1() { let _ = x.exp() - 1.0 * 2.0; } +fn check_log_division() { + let x = 3f32; + let y = 2f32; + let b = 4f32; + + let _ = x.log2() / y.log2(); + let _ = x.log10() / y.log10(); + let _ = x.ln() / y.ln(); + let _ = x.log(4.0) / y.log(4.0); + let _ = x.log(b) / y.log(b); + let _ = x.log(b) / y.log(x); + let _ = x.log(b) / 2f32.log(b); + + let x = 3f64; + let y = 2f64; + let b = 4f64; + + let _ = x.log2() / y.log2(); + let _ = x.log10() / y.log10(); + let _ = x.ln() / y.ln(); + let _ = x.log(4.0) / y.log(4.0); + let _ = x.log(b) / y.log(b); + let _ = x.log(b) / y.log(x); + let _ = x.log(b) / 2f64.log(b); +} + fn main() {} diff --git a/tests/ui/floating_point_arithmetic.stderr b/tests/ui/floating_point_arithmetic.stderr index 0fc05bce2528..076b8d4fefe6 100644 --- a/tests/ui/floating_point_arithmetic.stderr +++ b/tests/ui/floating_point_arithmetic.stderr @@ -192,5 +192,77 @@ error: (e.pow(x) - 1) can be computed more accurately LL | let _ = x.exp() - 1.0 + 2.0; | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` -error: aborting due to 32 previous errors +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:90:13 + | +LL | let _ = x.log2() / y.log2(); + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:91:13 + | +LL | let _ = x.log10() / y.log10(); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:92:13 + | +LL | let _ = x.ln() / y.ln(); + | ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:93:13 + | +LL | let _ = x.log(4.0) / y.log(4.0); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:94:13 + | +LL | let _ = x.log(b) / y.log(b); + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:96:13 + | +LL | let _ = x.log(b) / 2f32.log(b); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log2()` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:102:13 + | +LL | let _ = x.log2() / y.log2(); + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:103:13 + | +LL | let _ = x.log10() / y.log10(); + | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:104:13 + | +LL | let _ = x.ln() / y.ln(); + | ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:105:13 + | +LL | let _ = x.log(4.0) / y.log(4.0); + | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:106:13 + | +LL | let _ = x.log(b) / y.log(b); + | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` + +error: x.log(b) / y.log(b) can be reduced to x.log(y) + --> $DIR/floating_point_arithmetic.rs:108:13 + | +LL | let _ = x.log(b) / 2f64.log(b); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log2()` + +error: aborting due to 44 previous errors