diff --git a/CHANGELOG.md b/CHANGELOG.md index 962f9067a4e4..d084b6d6bb0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1033,6 +1033,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 @@ -1192,6 +1193,7 @@ Released 2018-09-13 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else +[`slow_floating_point_computation`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_floating_point_computation [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add diff --git a/README.md b/README.md index 6133fa4c3a57..054234cb7add 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 340 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 342 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs new file mode 100644 index 000000000000..935d8b581466 --- /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 736ff30c81a7..250e0fa3c7ac 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -200,6 +200,7 @@ pub mod excessive_precision; pub mod exit; pub mod explicit_write; pub mod fallible_impl_from; +pub mod floating_point_arithmetic; pub mod format; pub mod formatting; pub mod functions; @@ -520,6 +521,8 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &Conf &exit::EXIT, &explicit_write::EXPLICIT_WRITE, &fallible_impl_from::FALLIBLE_IMPL_FROM, + &floating_point_arithmetic::INACCURATE_FLOATING_POINT_COMPUTATION, + &floating_point_arithmetic::SLOW_FLOATING_POINT_COMPUTATION, &format::USELESS_FORMAT, &formatting::POSSIBLE_MISSING_COMMA, &formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING, @@ -968,6 +971,7 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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); @@ -1582,6 +1586,8 @@ pub fn register_plugins(store: &mut lint::LintStore, sess: &Session, conf: &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(&mutex_atomic::MUTEX_INTEGER), diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index f4ebf6cbd918..65c9ac8e4f0d 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 340] = [ +pub const ALL_LINTS: [Lint; 342] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -735,6 +735,13 @@ pub const ALL_LINTS: [Lint; 340] = [ 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", @@ -1813,6 +1820,13 @@ pub const ALL_LINTS: [Lint; 340] = [ deprecation: None, module: "matches", }, + Lint { + name: "slow_floating_point_computation", + group: "nursery", + desc: "checks for inefficient floating point computations", + deprecation: None, + module: "floating_point_arithmetic", + }, Lint { name: "slow_vector_initialization", group: "perf", diff --git a/tests/ui/floating_point_arithmetic.rs b/tests/ui/floating_point_arithmetic.rs new file mode 100644 index 000000000000..5291e5cf0226 --- /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 000000000000..a0663e488354 --- /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 +