Skip to content

Commit c636c6a

Browse files
krishna-veerareddykrishna-veerareddy
committed
Add lints to detect inaccurate and inefficient FP operations
Add lints to detect floating point computations that are either inaccurate or inefficient and suggest better alternatives.
1 parent fc5d0cc commit c636c6a

File tree

6 files changed

+489
-0
lines changed

6 files changed

+489
-0
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1169,6 +1169,7 @@ Released 2018-09-13
11691169
[`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond
11701170
[`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher
11711171
[`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return
1172+
[`inaccurate_floating_point_computation`]: https://rust-lang.github.io/rust-clippy/master/index.html#inaccurate_floating_point_computation
11721173
[`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
11731174
[`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
11741175
[`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask

‎clippy_lints/src/floating_point_arithmetic.rs

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
use crate::consts::{
2+
constant,
3+
Constant::{F32, F64},
4+
};
5+
use crate::utils::*;
6+
use if_chain::if_chain;
7+
use rustc::declare_lint_pass;
8+
use rustc::hir::*;
9+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
10+
use rustc_errors::Applicability;
11+
use rustc_session::declare_tool_lint;
12+
use std::f32::consts as f32_consts;
13+
use std::f64::consts as f64_consts;
14+
15+
declare_clippy_lint! {
16+
/// **What it does:** Looks for numerically unstable floating point
17+
/// computations and suggests better alternatives.
18+
///
19+
/// **Why is this bad?** Numerically unstable floating point computations
20+
/// cause rounding errors to magnify and distorts the results strongly.
21+
///
22+
/// **Known problems:** None
23+
///
24+
/// **Example:**
25+
///
26+
/// ```rust
27+
/// use std::f32::consts::E;
28+
///
29+
/// let a = 1f32.log(2.0);
30+
/// let b = 1f32.log(10.0);
31+
/// let c = 1f32.log(E);
32+
/// ```
33+
///
34+
/// is better expressed as
35+
///
36+
/// ```rust
37+
/// let a = 1f32.log2();
38+
/// let b = 1f32.log10();
39+
/// let c = 1f32.ln();
40+
/// ```
41+
pub INACCURATE_FLOATING_POINT_COMPUTATION,
42+
nursery,
43+
"checks for numerically unstable floating point computations"
44+
}
45+
46+
declare_clippy_lint! {
47+
/// **What it does:** Looks for inefficient floating point computations
48+
/// and suggests faster alternatives.
49+
///
50+
/// **Why is this bad?** Lower performance.
51+
///
52+
/// **Known problems:** None
53+
///
54+
/// **Example:**
55+
///
56+
/// ```rust
57+
/// use std::f32::consts::E;
58+
///
59+
/// let a = (2f32).powf(3.0);
60+
/// let c = E.powf(3.0);
61+
/// ```
62+
///
63+
/// is better expressed as
64+
///
65+
/// ```rust
66+
/// let a = (3f32).exp2();
67+
/// let b = (3f32).exp();
68+
/// ```
69+
pub SLOW_FLOATING_POINT_COMPUTATION,
70+
nursery,
71+
"checks for inefficient floating point computations"
72+
}
73+
74+
declare_lint_pass!(FloatingPointArithmetic => [
75+
INACCURATE_FLOATING_POINT_COMPUTATION,
76+
SLOW_FLOATING_POINT_COMPUTATION
77+
]);
78+
79+
fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec<Expr>) {
80+
let recv = &args[0];
81+
let arg = sugg::Sugg::hir(cx, recv, "..").maybe_par();
82+
83+
if let Some((value, _)) = constant(cx, cx.tables, &args[1]) {
84+
let method;
85+
86+
if F32(2.0) == value || F64(2.0) == value {
87+
method = "log2";
88+
} else if F32(10.0) == value || F64(10.0) == value {
89+
method = "log10";
90+
} else if F32(f32_consts::E) == value || F64(f64_consts::E) == value {
91+
method = "ln";
92+
} else {
93+
return;
94+
}
95+
96+
span_lint_and_sugg(
97+
cx,
98+
INACCURATE_FLOATING_POINT_COMPUTATION,
99+
expr.span,
100+
"logarithm for bases 2, 10 and e can be computed more accurately",
101+
"consider using",
102+
format!("{}.{}()", arg, method),
103+
Applicability::MachineApplicable,
104+
);
105+
}
106+
}
107+
108+
// TODO: Lint expressions of the form `(x + 1).ln()` and `(x + y).ln()`
109+
// where y > 1 and suggest usage of `(x + (y - 1)).ln_1p()` instead
110+
fn check_ln1p(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec<Expr>) {
111+
if_chain! {
112+
if let ExprKind::Binary(op, ref lhs, ref rhs) = &args[0].kind;
113+
if op.node == BinOpKind::Add;
114+
if let Some((value, _)) = constant(cx, cx.tables, lhs);
115+
if F32(1.0) == value || F64(1.0) == value;
116+
then {
117+
let arg = sugg::Sugg::hir(cx, rhs, "..").maybe_par();
118+
119+
span_lint_and_sugg(
120+
cx,
121+
INACCURATE_FLOATING_POINT_COMPUTATION,
122+
expr.span,
123+
"ln(1 + x) can be computed more accurately",
124+
"consider using",
125+
format!("{}.ln_1p()", arg),
126+
Applicability::MachineApplicable,
127+
);
128+
}
129+
}
130+
}
131+
132+
fn check_powf(cx: &LateContext<'_, '_>, expr: &Expr, args: &HirVec<Expr>) {
133+
// Check receiver
134+
if let Some((value, _)) = constant(cx, cx.tables, &args[0]) {
135+
let method;
136+
137+
if F32(f32_consts::E) == value || F64(f64_consts::E) == value {
138+
method = "exp";
139+
} else if F32(2.0) == value || F64(2.0) == value {
140+
method = "exp2";
141+
} else {
142+
return;
143+
}
144+
145+
span_lint_and_sugg(
146+
cx,
147+
SLOW_FLOATING_POINT_COMPUTATION,
148+
expr.span,
149+
"exponent for bases 2 and e can be computed more efficiently",
150+
"consider using",
151+
format!("{}.{}()", sugg::Sugg::hir(cx, &args[1], "..").maybe_par(), method),
152+
Applicability::MachineApplicable,
153+
);
154+
}
155+
156+
// Check argument
157+
if let Some((value, _)) = constant(cx, cx.tables, &args[1]) {
158+
let help;
159+
let method;
160+
161+
if F32(1.0 / 2.0) == value || F64(1.0 / 2.0) == value {
162+
help = "square-root of a number can be computer more efficiently";
163+
method = "sqrt";
164+
} else if F32(1.0 / 3.0) == value || F64(1.0 / 3.0) == value {
165+
help = "cube-root of a number can be computer more efficiently";
166+
method = "cbrt";
167+
} else {
168+
return;
169+
}
170+
171+
span_lint_and_sugg(
172+
cx,
173+
SLOW_FLOATING_POINT_COMPUTATION,
174+
expr.span,
175+
help,
176+
"consider using",
177+
format!("{}.{}()", sugg::Sugg::hir(cx, &args[0], ".."), method),
178+
Applicability::MachineApplicable,
179+
);
180+
}
181+
}
182+
183+
// TODO: Lint expressions of the form `x.exp() - y` where y > 1
184+
// and suggest usage of `x.exp_m1() - (y - 1)` instead
185+
fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr) {
186+
if_chain! {
187+
if let ExprKind::Binary(op, ref lhs, ref rhs) = expr.kind;
188+
if op.node == BinOpKind::Sub;
189+
if cx.tables.expr_ty(lhs).is_floating_point();
190+
if let Some((value, _)) = constant(cx, cx.tables, rhs);
191+
if F32(1.0) == value || F64(1.0) == value;
192+
if let ExprKind::MethodCall(ref path, _, ref method_args) = lhs.kind;
193+
if path.ident.name.as_str() == "exp";
194+
then {
195+
span_lint_and_sugg(
196+
cx,
197+
INACCURATE_FLOATING_POINT_COMPUTATION,
198+
expr.span,
199+
"(e.pow(x) - 1) can be computed more accurately",
200+
"consider using",
201+
format!(
202+
"{}.exp_m1()",
203+
sugg::Sugg::hir(cx, &method_args[0], "..")
204+
),
205+
Applicability::MachineApplicable,
206+
);
207+
}
208+
}
209+
}
210+
211+
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic {
212+
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
213+
if let ExprKind::MethodCall(ref path, _, args) = &expr.kind {
214+
let recv_ty = cx.tables.expr_ty(&args[0]);
215+
216+
if recv_ty.is_floating_point() {
217+
match &*path.ident.name.as_str() {
218+
"ln" => check_ln1p(cx, expr, args),
219+
"log" => check_log_base(cx, expr, args),
220+
"powf" => check_powf(cx, expr, args),
221+
_ => {},
222+
}
223+
}
224+
} else {
225+
check_expm1(cx, expr);
226+
}
227+
}
228+
}

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,6 +1000,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10001000
store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome);
10011001
let array_size_threshold = conf.array_size_threshold;
10021002
store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold));
1003+
store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic);
10031004
store.register_early_pass(|| box as_conversions::AsConversions);
10041005
store.register_early_pass(|| box utils::internal_lints::ProduceIce);
10051006
store.register_late_pass(|| box let_underscore::LetUnderscore);
@@ -1648,6 +1649,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16481649
store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
16491650
LintId::of(&attrs::EMPTY_LINE_AFTER_OUTER_ATTR),
16501651
LintId::of(&fallible_impl_from::FALLIBLE_IMPL_FROM),
1652+
LintId::of(&floating_point_arithmetic::INACCURATE_FLOATING_POINT_COMPUTATION),
1653+
LintId::of(&floating_point_arithmetic::SLOW_FLOATING_POINT_COMPUTATION),
16511654
LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN),
16521655
LintId::of(&mul_add::MANUAL_MUL_ADD),
16531656
LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL),

‎src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,13 @@ pub const ALL_LINTS: [Lint; 357] = [
749749
deprecation: None,
750750
module: "implicit_return",
751751
},
752+
Lint {
753+
name: "inaccurate_floating_point_computation",
754+
group: "nursery",
755+
desc: "checks for numerically unstable floating point computations",
756+
deprecation: None,
757+
module: "floating_point_arithmetic",
758+
},
752759
Lint {
753760
name: "inconsistent_digit_grouping",
754761
group: "style",

‎tests/ui/floating_point_arithmetic.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#![allow(dead_code)]
2+
#![warn(
3+
clippy::inaccurate_floating_point_computation,
4+
clippy::slow_floating_point_computation
5+
)]
6+
7+
const TWO: f32 = 2.0;
8+
const E: f32 = std::f32::consts::E;
9+
10+
fn check_log_base() {
11+
let x = 1f32;
12+
let _ = x.log(2f32);
13+
let _ = x.log(10f32);
14+
let _ = x.log(std::f32::consts::E);
15+
let _ = x.log(TWO);
16+
let _ = x.log(E);
17+
18+
let x = 1f64;
19+
let _ = x.log(2f64);
20+
let _ = x.log(10f64);
21+
let _ = x.log(std::f64::consts::E);
22+
}
23+
24+
fn check_ln1p() {
25+
let x = 1f32;
26+
let _ = (1.0 + x).ln();
27+
let _ = (1.0 + x * 2.0).ln();
28+
let _ = (1.0 + x.powi(2)).ln();
29+
let _ = (1.0 + x.powi(2) * 2.0).ln();
30+
let _ = (1.0 + (std::f32::consts::E - 1.0)).ln();
31+
// Cases where the lint shouldn't be applied
32+
let _ = (x + 1.0).ln();
33+
let _ = (1.0 + x + 2.0).ln();
34+
let _ = (1.0 + x - 2.0).ln();
35+
36+
let x = 1f64;
37+
let _ = (1.0 + x).ln();
38+
let _ = (1.0 + x * 2.0).ln();
39+
let _ = (1.0 + x.powi(2)).ln();
40+
// Cases where the lint shouldn't be applied
41+
let _ = (x + 1.0).ln();
42+
let _ = (1.0 + x + 2.0).ln();
43+
let _ = (1.0 + x - 2.0).ln();
44+
}
45+
46+
fn check_powf() {
47+
let x = 3f32;
48+
let _ = 2f32.powf(x);
49+
let _ = std::f32::consts::E.powf(x);
50+
let _ = x.powf(1.0 / 2.0);
51+
let _ = x.powf(1.0 / 3.0);
52+
53+
let x = 3f64;
54+
let _ = 2f64.powf(x);
55+
let _ = std::f64::consts::E.powf(x);
56+
let _ = x.powf(1.0 / 2.0);
57+
let _ = x.powf(1.0 / 3.0);
58+
}
59+
60+
fn check_expm1() {
61+
let x = 2f32;
62+
let _ = x.exp() - 1.0;
63+
let _ = x.exp() - 1.0 + 2.0;
64+
// Cases where the lint shouldn't be applied
65+
let _ = x.exp() - 2.0;
66+
let _ = x.exp() - 1.0 * 2.0;
67+
68+
let x = 2f64;
69+
let _ = x.exp() - 1.0;
70+
let _ = x.exp() - 1.0 + 2.0;
71+
// Cases where the lint shouldn't be applied
72+
let _ = x.exp() - 2.0;
73+
let _ = x.exp() - 1.0 * 2.0;
74+
}
75+
76+
fn main() {}

0 commit comments

Comments
 (0)