dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Finished checking for cases of absolute values

This commit is contained in:
JarredAllen 2020-02-29 13:46:59 -08:00
parent 5a21661ce5
commit 028cddb956
3 changed files with 145 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

75
clippy_lints/src/floating_point_arithmetic.rs
View File

@ -387,6 +387,18 @@ fn is_testing_positive(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_
}
}
fn is_testing_negative(cx: &LateContext<'_, '_>, expr: &Expr<'_>, test: &Expr<'_>) -> bool {
if let ExprKind::Binary(Spanned { node: op, .. }, left, right) = expr.kind {
match op {
BinOpKind::Gt | BinOpKind::Ge => is_zero(left) && are_exprs_equal(cx, right, test),
BinOpKind::Lt | BinOpKind::Le => is_zero(right) && are_exprs_equal(cx, left, test),
_ => false,
}
} else {
false
}
}
fn are_exprs_equal(cx: &LateContext<'_, '_>, expr1: &Expr<'_>, expr2: &Expr<'_>) -> bool {
SpanlessEq::new(cx).ignore_fn().eq_expr(expr1, expr2)
}
@ -410,30 +422,9 @@ fn is_zero(expr: &Expr<'_>) -> bool {
fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
if let Some((cond, body, Some(else_body))) = higher::if_block(&expr) {
if let ExprKind::Block(
Block {
stmts: [],
expr:
Some(Expr {
kind: ExprKind::Unary(UnOp::UnNeg, else_expr),
..
}),
..
},
_,
) = else_body.kind
{
if let ExprKind::Block(
Block {
stmts: [],
expr: Some(body),
..
},
_,
) = &body.kind
{
if let ExprKind::Block( Block { stmts: [], expr: Some(Expr { kind: ExprKind::Unary(UnOp::UnNeg, else_expr), .. }), .. }, _,) = else_body.kind {
if let ExprKind::Block( Block { stmts: [], expr: Some(body), .. }, _,) = &body.kind {
if are_exprs_equal(cx, else_expr, body) {
dbg!("if (cond) body else -body\nbody: {:?}", &body.kind);
if is_testing_positive(cx, cond, body) {
span_lint_and_sugg(
cx,
@ -444,6 +435,44 @@ fn check_custom_abs(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
format!("{}.abs()", Sugg::hir(cx, body, "..")),
Applicability::MachineApplicable,
);
} else if is_testing_negative(cx, cond, body) {
span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"This looks like you've implemented your own negative absolute value function",
"try",
format!("-{}.abs()", Sugg::hir(cx, body, "..")),
Applicability::MachineApplicable,
);
}
}
}
}
if let ExprKind::Block( Block { stmts: [], expr: Some(Expr { kind: ExprKind::Unary(UnOp::UnNeg, else_expr), .. }), .. }, _,) = &body.kind
{
if let ExprKind::Block( Block { stmts: [], expr: Some(body), .. }, _,) = &else_body.kind {
if are_exprs_equal(cx, else_expr, body) {
if is_testing_negative(cx, cond, body) {
span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"This looks like you've implemented your own absolute value function",
"try",
format!("{}.abs()", Sugg::hir(cx, body, "..")),
Applicability::MachineApplicable,
);
} else if is_testing_positive(cx, cond, body) {
span_lint_and_sugg(
cx,
SUBOPTIMAL_FLOPS,
expr.span,
"This looks like you've implemented your own negative absolute value function",
"try",
format!("-{}.abs()", Sugg::hir(cx, body, "..")),
Applicability::MachineApplicable,
);
}
}
}

23
tests/ui/floating_point_abs.rs
View File

@ -1,4 +1,5 @@
#[warn(clippy::suboptimal_flops)]
#![warn(clippy::suboptimal_flops)]
struct A {
a: f64,
b: f64
@ -28,6 +29,22 @@ fn fake_abs3(a: A) -> f64 {
}
}
fn fake_abs4(num: f64) -> f64 {
if 0.0 >= num {
-num
} else {
num
}
}
fn fake_abs5(a: A) -> f64 {
if a.a < 0.0 {
-a.a
} else {
a.a
}
}
fn fake_nabs1(num: f64) -> f64 {
if num < 0.0 {
num
@ -46,9 +63,9 @@ fn fake_nabs2(num: f64) -> f64 {
fn fake_nabs3(a: A) -> A {
A { a: if a.a >= 0.0 {
a.a
} else {
-a.a
} else {
a.a
}, b: a.b }
}

75
tests/ui/floating_point_abs.stderr
View File

@ -1,5 +1,5 @@
error: This looks like you've implemented your own absolute value function
--> $DIR/floating_point_abs.rs:4:5
--> $DIR/floating_point_abs.rs:9:5
|
LL | / if num >= 0.0 {
LL | | num
@ -10,5 +10,76 @@ LL | | }
|
= note: `-D clippy::suboptimal-flops` implied by `-D warnings`
error: aborting due to previous error
error: This looks like you've implemented your own absolute value function
--> $DIR/floating_point_abs.rs:17:5
|
LL | / if 0.0 < num {
LL | | num
LL | | } else {
LL | | -num
LL | | }
| |_____^ help: try: `num.abs()`
error: This looks like you've implemented your own absolute value function
--> $DIR/floating_point_abs.rs:25:5
|
LL | / if a.a > 0.0 {
LL | | a.a
LL | | } else {
LL | | -a.a
LL | | }
| |_____^ help: try: `a.a.abs()`
error: This looks like you've implemented your own absolute value function
--> $DIR/floating_point_abs.rs:33:5
|
LL | / if 0.0 >= num {
LL | | -num
LL | | } else {
LL | | num
LL | | }
| |_____^ help: try: `num.abs()`
error: This looks like you've implemented your own absolute value function
--> $DIR/floating_point_abs.rs:41:5
|
LL | / if a.a < 0.0 {
LL | | -a.a
LL | | } else {
LL | | a.a
LL | | }
| |_____^ help: try: `a.a.abs()`
error: This looks like you've implemented your own negative absolute value function
--> $DIR/floating_point_abs.rs:49:5
|
LL | / if num < 0.0 {
LL | | num
LL | | } else {
LL | | -num
LL | | }
| |_____^ help: try: `-num.abs()`
error: This looks like you've implemented your own negative absolute value function
--> $DIR/floating_point_abs.rs:57:5
|
LL | / if 0.0 >= num {
LL | | num
LL | | } else {
LL | | -num
LL | | }
| |_____^ help: try: `-num.abs()`
error: This looks like you've implemented your own negative absolute value function
--> $DIR/floating_point_abs.rs:65:12
|
LL | A { a: if a.a >= 0.0 {
| ____________^
LL | | -a.a
LL | | } else {
LL | | a.a
LL | | }, b: a.b }
| |_________^ help: try: `-a.a.abs()`
error: aborting due to 8 previous errors