diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e99342e4..55e46bf6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1169,6 +1169,7 @@ Released 2018-09-13 [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return +[`inaccurate_floating_point_computation`]: https://rust-lang.github.io/rust-clippy/master/index.html#inaccurate_floating_point_computation [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs new file mode 100644 index 000000000..935d8b581 --- /dev/null +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -0,0 +1,228 @@ +use crate::consts::{ + constant, + Constant::{F32, F64}, +}; +use crate::utils::*; +use if_chain::if_chain; +use rustc::declare_lint_pass; +use rustc::hir::*; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc_errors::Applicability; +use rustc_session::declare_tool_lint; +use std::f32::consts as f32_consts; +use std::f64::consts as f64_consts; + +declare_clippy_lint! { + /// **What it does:** Looks for numerically unstable floating point + /// computations and suggests better alternatives. + /// + /// **Why is this bad?** Numerically unstable floating point computations + /// cause rounding errors to magnify and distorts the results strongly. + /// + /// **Known problems:** None + /// + /// **Example:** + /// + /// ```rust + /// use std::f32::consts::E; + /// + /// let a = 1f32.log(2.0); + /// let b = 1f32.log(10.0); + /// let c = 1f32.log(E); + /// ``` + /// + /// is better expressed as + /// + /// ```rust + /// let a = 1f32.log2(); + /// let b = 1f32.log10(); + /// let c = 1f32.ln(); + /// ``` + pub INACCURATE_FLOATING_POINT_COMPUTATION, + nursery, + "checks for numerically unstable floating point computations" +} + +declare_clippy_lint! { + /// **What it does:** Looks for inefficient floating point computations + /// and suggests faster alternatives. + /// + /// **Why is this bad?** Lower performance. + /// + /// **Known problems:** None + /// + /// **Example:** + /// + /// ```rust + /// use std::f32::consts::E; + /// + /// let a = (2f32).powf(3.0); + /// let c = E.powf(3.0); + /// ``` + /// + /// is better expressed as + /// + /// ```rust + /// let a = (3f32).exp2(); + /// let b = (3f32).exp(); + /// ``` + pub SLOW_FLOATING_POINT_COMPUTATION, + nursery, + "checks for inefficient floating point computations" +} + +declare_lint_pass!(FloatingPointArithmetic => [ + INACCURATE_FLOATING_POINT_COMPUTATION, + SLOW_FLOATING_POINT_COMPUTATION +]); + +fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec) { + let recv = &args[0]; + let arg = sugg::Sugg::hir(cx, recv, "..").maybe_par(); + + if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { + let method; + + if F32(2.0) == value || F64(2.0) == value { + method = "log2"; + } else if F32(10.0) == value || F64(10.0) == value { + method = "log10"; + } else if F32(f32_consts::E) == value || F64(f64_consts::E) == value { + method = "ln"; + } else { + return; + } + + span_lint_and_sugg( + cx, + INACCURATE_FLOATING_POINT_COMPUTATION, + expr.span, + "logarithm for bases 2, 10 and e can be computed more accurately", + "consider using", + format!("{}.{}()", arg, method), + Applicability::MachineApplicable, + ); + } +} + +// TODO: Lint expressions of the form `(x + 1).ln()` and `(x + y).ln()` +// where y > 1 and suggest usage of `(x + (y - 1)).ln_1p()` instead +fn check_ln1p(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec) { + if_chain! { + if let ExprKind::Binary(op, ref lhs, ref rhs) = &args[0].kind; + if op.node == BinOpKind::Add; + if let Some((value, _)) = constant(cx, cx.tables, lhs); + if F32(1.0) == value || F64(1.0) == value; + then { + let arg = sugg::Sugg::hir(cx, rhs, "..").maybe_par(); + + span_lint_and_sugg( + cx, + INACCURATE_FLOATING_POINT_COMPUTATION, + expr.span, + "ln(1 + x) can be computed more accurately", + "consider using", + format!("{}.ln_1p()", arg), + Applicability::MachineApplicable, + ); + } + } +} + +fn check_powf(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec) { + // Check receiver + if let Some((value, _)) = constant(cx, cx.tables, &args[0]) { + let method; + + if F32(f32_consts::E) == value || F64(f64_consts::E) == value { + method = "exp"; + } else if F32(2.0) == value || F64(2.0) == value { + method = "exp2"; + } else { + return; + } + + span_lint_and_sugg( + cx, + SLOW_FLOATING_POINT_COMPUTATION, + expr.span, + "exponent for bases 2 and e can be computed more efficiently", + "consider using", + format!("{}.{}()", sugg::Sugg::hir(cx, &args[1], "..").maybe_par(), method), + Applicability::MachineApplicable, + ); + } + + // Check argument + if let Some((value, _)) = constant(cx, cx.tables, &args[1]) { + let help; + let method; + + if F32(1.0 / 2.0) == value || F64(1.0 / 2.0) == value { + help = "square-root of a number can be computer more efficiently"; + method = "sqrt"; + } else if F32(1.0 / 3.0) == value || F64(1.0 / 3.0) == value { + help = "cube-root of a number can be computer more efficiently"; + method = "cbrt"; + } else { + return; + } + + span_lint_and_sugg( + cx, + SLOW_FLOATING_POINT_COMPUTATION, + expr.span, + help, + "consider using", + format!("{}.{}()", sugg::Sugg::hir(cx, &args[0], ".."), method), + 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) { + if_chain! { + if let ExprKind::Binary(op, ref lhs, ref rhs) = expr.kind; + if op.node == BinOpKind::Sub; + if cx.tables.expr_ty(lhs).is_floating_point(); + if let Some((value, _)) = constant(cx, cx.tables, rhs); + if F32(1.0) == value || F64(1.0) == value; + if let ExprKind::MethodCall(ref path, _, ref method_args) = lhs.kind; + if path.ident.name.as_str() == "exp"; + then { + span_lint_and_sugg( + cx, + INACCURATE_FLOATING_POINT_COMPUTATION, + expr.span, + "(e.pow(x) - 1) can be computed more accurately", + "consider using", + format!( + "{}.exp_m1()", + sugg::Sugg::hir(cx, &method_args[0], "..") + ), + 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 { + let recv_ty = cx.tables.expr_ty(&args[0]); + + if recv_ty.is_floating_point() { + match &*path.ident.name.as_str() { + "ln" => check_ln1p(cx, expr, args), + "log" => check_log_base(cx, expr, args), + "powf" => check_powf(cx, expr, args), + _ => {}, + } + } + } else { + check_expm1(cx, expr); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2b5691f92..9a904141f 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1000,6 +1000,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome); let array_size_threshold = conf.array_size_threshold; store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold)); + store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic); store.register_early_pass(|| box as_conversions::AsConversions); store.register_early_pass(|| box utils::internal_lints::ProduceIce); store.register_late_pass(|| box let_underscore::LetUnderscore); @@ -1648,6 +1649,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(&attrs::EMPTY_LINE_AFTER_OUTER_ATTR), LintId::of(&fallible_impl_from::FALLIBLE_IMPL_FROM), + LintId::of(&floating_point_arithmetic::INACCURATE_FLOATING_POINT_COMPUTATION), + LintId::of(&floating_point_arithmetic::SLOW_FLOATING_POINT_COMPUTATION), LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(&mul_add::MANUAL_MUL_ADD), LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 29b5a7ba0..fee595074 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -749,6 +749,13 @@ pub const ALL_LINTS: [Lint; 357] = [ deprecation: None, module: "implicit_return", }, + Lint { + name: "inaccurate_floating_point_computation", + group: "nursery", + desc: "checks for numerically unstable floating point computations", + deprecation: None, + module: "floating_point_arithmetic", + }, Lint { name: "inconsistent_digit_grouping", group: "style", diff --git a/tests/ui/floating_point_arithmetic.rs b/tests/ui/floating_point_arithmetic.rs new file mode 100644 index 000000000..5291e5cf0 --- /dev/null +++ b/tests/ui/floating_point_arithmetic.rs @@ -0,0 +1,76 @@ +#![allow(dead_code)] +#![warn( + clippy::inaccurate_floating_point_computation, + clippy::slow_floating_point_computation +)] + +const TWO: f32 = 2.0; +const E: f32 = std::f32::consts::E; + +fn check_log_base() { + let x = 1f32; + let _ = x.log(2f32); + let _ = x.log(10f32); + let _ = x.log(std::f32::consts::E); + let _ = x.log(TWO); + let _ = x.log(E); + + let x = 1f64; + let _ = x.log(2f64); + let _ = x.log(10f64); + let _ = x.log(std::f64::consts::E); +} + +fn check_ln1p() { + let x = 1f32; + 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 + (std::f32::consts::E - 1.0)).ln(); + // Cases where the lint shouldn't be applied + let _ = (x + 1.0).ln(); + let _ = (1.0 + x + 2.0).ln(); + let _ = (1.0 + x - 2.0).ln(); + + let x = 1f64; + let _ = (1.0 + x).ln(); + let _ = (1.0 + x * 2.0).ln(); + let _ = (1.0 + x.powi(2)).ln(); + // Cases where the lint shouldn't be applied + let _ = (x + 1.0).ln(); + let _ = (1.0 + x + 2.0).ln(); + let _ = (1.0 + x - 2.0).ln(); +} + +fn check_powf() { + let x = 3f32; + let _ = 2f32.powf(x); + let _ = std::f32::consts::E.powf(x); + let _ = x.powf(1.0 / 2.0); + let _ = x.powf(1.0 / 3.0); + + let x = 3f64; + let _ = 2f64.powf(x); + let _ = std::f64::consts::E.powf(x); + let _ = x.powf(1.0 / 2.0); + let _ = x.powf(1.0 / 3.0); +} + +fn check_expm1() { + let x = 2f32; + let _ = x.exp() - 1.0; + let _ = x.exp() - 1.0 + 2.0; + // Cases where the lint shouldn't be applied + let _ = x.exp() - 2.0; + let _ = x.exp() - 1.0 * 2.0; + + let x = 2f64; + let _ = x.exp() - 1.0; + let _ = x.exp() - 1.0 + 2.0; + // Cases where the lint shouldn't be applied + let _ = x.exp() - 2.0; + let _ = x.exp() - 1.0 * 2.0; +} + +fn main() {} diff --git a/tests/ui/floating_point_arithmetic.stderr b/tests/ui/floating_point_arithmetic.stderr new file mode 100644 index 000000000..a0663e488 --- /dev/null +++ b/tests/ui/floating_point_arithmetic.stderr @@ -0,0 +1,174 @@ +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:12:13 + | +LL | let _ = x.log(2f32); + | ^^^^^^^^^^^ help: consider using: `x.log2()` + | + = note: `-D clippy::inaccurate-floating-point-computation` implied by `-D warnings` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:13:13 + | +LL | let _ = x.log(10f32); + | ^^^^^^^^^^^^ help: consider using: `x.log10()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:14:13 + | +LL | let _ = x.log(std::f32::consts::E); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.ln()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:15:13 + | +LL | let _ = x.log(TWO); + | ^^^^^^^^^^ help: consider using: `x.log2()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:16:13 + | +LL | let _ = x.log(E); + | ^^^^^^^^ help: consider using: `x.ln()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:19:13 + | +LL | let _ = x.log(2f64); + | ^^^^^^^^^^^ help: consider using: `x.log2()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:20:13 + | +LL | let _ = x.log(10f64); + | ^^^^^^^^^^^^ help: consider using: `x.log10()` + +error: logarithm for bases 2, 10 and e can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:21:13 + | +LL | let _ = x.log(std::f64::consts::E); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.ln()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:26:13 + | +LL | let _ = (1.0 + x).ln(); + | ^^^^^^^^^^^^^^ help: consider using: `x.ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:27:13 + | +LL | let _ = (1.0 + x * 2.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x * 2.0).ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:28:13 + | +LL | let _ = (1.0 + x.powi(2)).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:29:13 + | +LL | let _ = (1.0 + x.powi(2) * 2.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x.powi(2) * 2.0).ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:30:13 + | +LL | let _ = (1.0 + (std::f32::consts::E - 1.0)).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `((std::f32::consts::E - 1.0)).ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:37:13 + | +LL | let _ = (1.0 + x).ln(); + | ^^^^^^^^^^^^^^ help: consider using: `x.ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:38:13 + | +LL | let _ = (1.0 + x * 2.0).ln(); + | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x * 2.0).ln_1p()` + +error: ln(1 + x) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:39:13 + | +LL | let _ = (1.0 + x.powi(2)).ln(); + | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2).ln_1p()` + +error: exponent for bases 2 and e can be computed more efficiently + --> $DIR/floating_point_arithmetic.rs:48:13 + | +LL | let _ = 2f32.powf(x); + | ^^^^^^^^^^^^ help: consider using: `x.exp2()` + | + = note: `-D clippy::slow-floating-point-computation` implied by `-D warnings` + +error: exponent for bases 2 and e can be computed more efficiently + --> $DIR/floating_point_arithmetic.rs:49:13 + | +LL | let _ = std::f32::consts::E.powf(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.exp()` + +error: square-root of a number can be computer more efficiently + --> $DIR/floating_point_arithmetic.rs:50:13 + | +LL | let _ = x.powf(1.0 / 2.0); + | ^^^^^^^^^^^^^^^^^ help: consider using: `x.sqrt()` + +error: cube-root of a number can be computer more efficiently + --> $DIR/floating_point_arithmetic.rs:51:13 + | +LL | let _ = x.powf(1.0 / 3.0); + | ^^^^^^^^^^^^^^^^^ help: consider using: `x.cbrt()` + +error: exponent for bases 2 and e can be computed more efficiently + --> $DIR/floating_point_arithmetic.rs:54:13 + | +LL | let _ = 2f64.powf(x); + | ^^^^^^^^^^^^ help: consider using: `x.exp2()` + +error: exponent for bases 2 and e can be computed more efficiently + --> $DIR/floating_point_arithmetic.rs:55:13 + | +LL | let _ = std::f64::consts::E.powf(x); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.exp()` + +error: square-root of a number can be computer more efficiently + --> $DIR/floating_point_arithmetic.rs:56:13 + | +LL | let _ = x.powf(1.0 / 2.0); + | ^^^^^^^^^^^^^^^^^ help: consider using: `x.sqrt()` + +error: cube-root of a number can be computer more efficiently + --> $DIR/floating_point_arithmetic.rs:57:13 + | +LL | let _ = x.powf(1.0 / 3.0); + | ^^^^^^^^^^^^^^^^^ help: consider using: `x.cbrt()` + +error: (e.pow(x) - 1) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:62:13 + | +LL | let _ = x.exp() - 1.0; + | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` + +error: (e.pow(x) - 1) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:63:13 + | +LL | let _ = x.exp() - 1.0 + 2.0; + | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` + +error: (e.pow(x) - 1) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:69:13 + | +LL | let _ = x.exp() - 1.0; + | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` + +error: (e.pow(x) - 1) can be computed more accurately + --> $DIR/floating_point_arithmetic.rs:70:13 + | +LL | let _ = x.exp() - 1.0 + 2.0; + | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` + +error: aborting due to 28 previous errors +