Skip to content

Add lint to improve floating-point expressions #4897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename mul_add test file and add general improvements
  • Loading branch information
krishna-veerareddy committed Feb 24, 2020
commit e94a167508604d4f4d37bbd654fc3a39056ef99b
108 changes: 57 additions & 51 deletions clippy_lints/src/floating_point_arithmetic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Spanned;

use std::f32::consts as f32_consts;
use std::f64::consts as f64_consts;
use sugg::{format_numeric_literal, Sugg};
Expand Down Expand Up @@ -138,26 +140,29 @@ fn check_log_base(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>])
// TODO: Lint expressions of the form `(x + y).ln()` where y > 1 and
// suggest usage of `(x + (y - 1)).ln_1p()` instead
fn check_ln1p(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
if_chain! {
if let ExprKind::Binary(op, ref lhs, ref rhs) = &args[0].kind;
if op.node == BinOpKind::Add;
then {
let recv = match (constant(cx, cx.tables, lhs), constant(cx, cx.tables, rhs)) {
(Some((value, _)), _) if F32(1.0) == value || F64(1.0) == value => rhs,
(_, Some((value, _))) if F32(1.0) == value || F64(1.0) == value => lhs,
_ => return,
};
if let ExprKind::Binary(
Spanned {
node: BinOpKind::Add, ..
},
lhs,
rhs,
) = &args[0].kind
{
let recv = match (constant(cx, cx.tables, lhs), constant(cx, cx.tables, rhs)) {
(Some((value, _)), _) if F32(1.0) == value || F64(1.0) == value => rhs,
(_, Some((value, _))) if F32(1.0) == value || F64(1.0) == value => lhs,
_ => return,
};

span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"ln(1 + x) can be computed more accurately",
"consider using",
format!("{}.ln_1p()", prepare_receiver_sugg(cx, recv)),
Applicability::MachineApplicable,
);
}
span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"ln(1 + x) can be computed more accurately",
"consider using",
format!("{}.ln_1p()", prepare_receiver_sugg(cx, recv)),
Applicability::MachineApplicable,
);
}
}

Expand Down Expand Up @@ -249,8 +254,7 @@ fn check_powf(cx: &LateContext<'_, '_>, expr: &Expr<'_>, args: &[Expr<'_>]) {
// 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 let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, ref lhs, ref rhs) = expr.kind;
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;
Expand All @@ -276,8 +280,7 @@ fn check_expm1(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {

fn is_float_mul_expr<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option<(&'a Expr<'a>, &'a Expr<'a>)> {
if_chain! {
if let ExprKind::Binary(op, ref lhs, ref rhs) = &expr.kind;
if let BinOpKind::Mul = op.node;
if let ExprKind::Binary(Spanned { node: BinOpKind::Mul, .. }, ref lhs, ref rhs) = &expr.kind;
if cx.tables.expr_ty(lhs).is_floating_point();
if cx.tables.expr_ty(rhs).is_floating_point();
then {
Expand All @@ -289,34 +292,37 @@ fn is_float_mul_expr<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option
}

// TODO: Fix rust-lang/rust-clippy#4735
fn check_fma(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if_chain! {
if let ExprKind::Binary(op, lhs, rhs) = &expr.kind;
if let BinOpKind::Add = op.node;
then {
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) {
(inner_lhs, inner_rhs, lhs)
} else {
return;
};
fn check_mul_add(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if let ExprKind::Binary(
Spanned {
node: BinOpKind::Add, ..
},
lhs,
rhs,
) = &expr.kind
{
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) {
(inner_lhs, inner_rhs, lhs)
} else {
return;
};

span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"multiply and add expressions can be calculated more efficiently and accurately",
"consider using",
format!(
"{}.mul_add({}, {})",
prepare_receiver_sugg(cx, recv),
Sugg::hir(cx, arg1, ".."),
Sugg::hir(cx, arg2, ".."),
),
Applicability::MachineApplicable,
);
}
span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"multiply and add expressions can be calculated more efficiently and accurately",
"consider using",
format!(
"{}.mul_add({}, {})",
prepare_receiver_sugg(cx, recv),
Sugg::hir(cx, arg1, ".."),
Sugg::hir(cx, arg2, ".."),
),
Applicability::MachineApplicable,
);
}
}

Expand All @@ -335,7 +341,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FloatingPointArithmetic {
}
} else {
check_expm1(cx, expr);
check_fma(cx, expr);
check_mul_add(cx, expr);
}
}
}
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,55 +1,55 @@
error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:10:13
--> $DIR/floating_point_mul_add.rs:10:13
|
LL | let _ = a * b + c;
| ^^^^^^^^^ help: consider using: `a.mul_add(b, c)`
|
= note: `-D clippy::suboptimal-flops` implied by `-D warnings`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:11:13
--> $DIR/floating_point_mul_add.rs:11:13
|
LL | let _ = c + a * b;
| ^^^^^^^^^ help: consider using: `a.mul_add(b, c)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:12:13
--> $DIR/floating_point_mul_add.rs:12:13
|
LL | let _ = a + 2.0 * 4.0;
| ^^^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4.0, a)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:13:13
--> $DIR/floating_point_mul_add.rs:13:13
|
LL | let _ = a + 2. * 4.;
| ^^^^^^^^^^^ help: consider using: `2.0f64.mul_add(4., a)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:15:13
--> $DIR/floating_point_mul_add.rs:15:13
|
LL | let _ = (a * b) + c;
| ^^^^^^^^^^^ help: consider using: `a.mul_add(b, c)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:16:13
--> $DIR/floating_point_mul_add.rs:16:13
|
LL | let _ = c + (a * b);
| ^^^^^^^^^^^ help: consider using: `a.mul_add(b, c)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:17:13
--> $DIR/floating_point_mul_add.rs:17:13
|
LL | let _ = a * b * c + d;
| ^^^^^^^^^^^^^ help: consider using: `(a * b).mul_add(c, d)`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:19:13
--> $DIR/floating_point_mul_add.rs:19:13
|
LL | let _ = a.mul_add(b, c) * a.mul_add(b, c) + a.mul_add(b, c) + c;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `a.mul_add(b, c).mul_add(a.mul_add(b, c), a.mul_add(b, c))`

error: multiply and add expressions can be calculated more efficiently and accurately
--> $DIR/floating_point_fma.rs:20:13
--> $DIR/floating_point_mul_add.rs:20:13
|
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)`
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/floating_point_powf.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ fn main() {
let _ = x.powi(-2);
let _ = x.powi(16_777_215);
let _ = x.powi(-16_777_215);
// Cases where the lint shouldn't be applied
let _ = x.powf(2.1);
let _ = x.powf(-2.1);
let _ = x.powf(16_777_216.0);
Expand All @@ -33,6 +34,7 @@ fn main() {
let _ = x.powi(-2);
let _ = x.powi(-2_147_483_648);
let _ = x.powi(2_147_483_647);
// Cases where the lint shouldn't be applied
let _ = x.powf(2.1);
let _ = x.powf(-2.1);
let _ = x.powf(-2_147_483_649.0);
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/floating_point_powf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ fn main() {
let _ = x.powf(-2.0);
let _ = x.powf(16_777_215.0);
let _ = x.powf(-16_777_215.0);
// Cases where the lint shouldn't be applied
let _ = x.powf(2.1);
let _ = x.powf(-2.1);
let _ = x.powf(16_777_216.0);
Expand All @@ -33,6 +34,7 @@ fn main() {
let _ = x.powf(-2.0);
let _ = x.powf(-2_147_483_648.0);
let _ = x.powf(2_147_483_647.0);
// Cases where the lint shouldn't be applied
let _ = x.powf(2.1);
let _ = x.powf(-2.1);
let _ = x.powf(-2_147_483_649.0);
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/floating_point_powf.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -73,73 +73,73 @@ LL | let _ = x.powf(-16_777_215.0);
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(-16_777_215)`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:24:13
--> $DIR/floating_point_powf.rs:25:13
|
LL | let _ = 2f64.powf(x);
| ^^^^^^^^^^^^ help: consider using: `x.exp2()`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:25:13
--> $DIR/floating_point_powf.rs:26:13
|
LL | let _ = 2f64.powf(3.1);
| ^^^^^^^^^^^^^^ help: consider using: `3.1f64.exp2()`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:26:13
--> $DIR/floating_point_powf.rs:27:13
|
LL | let _ = 2f64.powf(-3.1);
| ^^^^^^^^^^^^^^^ help: consider using: `(-3.1f64).exp2()`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:27:13
--> $DIR/floating_point_powf.rs:28:13
|
LL | let _ = std::f64::consts::E.powf(x);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.exp()`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:28:13
--> $DIR/floating_point_powf.rs:29:13
|
LL | let _ = std::f64::consts::E.powf(3.1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `3.1f64.exp()`

error: exponent for bases 2 and e can be computed more accurately
--> $DIR/floating_point_powf.rs:29:13
--> $DIR/floating_point_powf.rs:30:13
|
LL | let _ = std::f64::consts::E.powf(-3.1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(-3.1f64).exp()`

error: square-root of a number can be computed more efficiently and accurately
--> $DIR/floating_point_powf.rs:30:13
--> $DIR/floating_point_powf.rs:31:13
|
LL | let _ = x.powf(1.0 / 2.0);
| ^^^^^^^^^^^^^^^^^ help: consider using: `x.sqrt()`

error: cube-root of a number can be computed more accurately
--> $DIR/floating_point_powf.rs:31:13
--> $DIR/floating_point_powf.rs:32:13
|
LL | let _ = x.powf(1.0 / 3.0);
| ^^^^^^^^^^^^^^^^^ help: consider using: `x.cbrt()`

error: exponentiation with integer powers can be computed more efficiently
--> $DIR/floating_point_powf.rs:32:13
--> $DIR/floating_point_powf.rs:33:13
|
LL | let _ = x.powf(2.0);
| ^^^^^^^^^^^ help: consider using: `x.powi(2)`

error: exponentiation with integer powers can be computed more efficiently
--> $DIR/floating_point_powf.rs:33:13
--> $DIR/floating_point_powf.rs:34:13
|
LL | let _ = x.powf(-2.0);
| ^^^^^^^^^^^^ help: consider using: `x.powi(-2)`

error: exponentiation with integer powers can be computed more efficiently
--> $DIR/floating_point_powf.rs:34:13
--> $DIR/floating_point_powf.rs:35:13
|
LL | let _ = x.powf(-2_147_483_648.0);
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(-2_147_483_648)`

error: exponentiation with integer powers can be computed more efficiently
--> $DIR/floating_point_powf.rs:35:13
--> $DIR/floating_point_powf.rs:36:13
|
LL | let _ = x.powf(2_147_483_647.0);
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2_147_483_647)`
Expand Down