improve identity_op
fix Update identity_op.rs ok update without_loop_counters test chore fix chore remove visitor and leftmost fix add document
This commit is contained in:
tamaron
parent
a9d31e71be
commit
6ad810f94e
@ -1,3 +1,4 @@
|
||||
use clippy_utils::get_parent_expr;
|
||||
use clippy_utils::source::snippet;
|
||||
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
@ -22,6 +23,11 @@ declare_clippy_lint! {
|
||||
/// # let x = 1;
|
||||
/// x / 1 + 0 * 1 - 0 | 0;
|
||||
/// ```
|
||||
///
|
||||
/// ### Known problems
|
||||
/// False negatives: `f(0 + if b { 1 } else { 2 } + 3);` is reducible to
|
||||
/// `f(if b { 1 } else { 2 } + 3);`. But the lint doesn't trigger for the code.
|
||||
/// See [#8724](https://github.com/rust-lang/rust-clippy/issues/8724)
|
||||
#[clippy::version = "pre 1.29.0"]
|
||||
pub IDENTITY_OP,
|
||||
complexity,
|
||||
@ -31,36 +37,66 @@ declare_clippy_lint! {
|
||||
declare_lint_pass!(IdentityOp => [IDENTITY_OP]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for IdentityOp {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
|
||||
if e.span.from_expansion() {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
if expr.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
if let ExprKind::Binary(cmp, left, right) = e.kind {
|
||||
if is_allowed(cx, cmp, left, right) {
|
||||
return;
|
||||
}
|
||||
match cmp.node {
|
||||
BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
|
||||
check(cx, left, 0, e.span, right.span);
|
||||
check(cx, right, 0, e.span, left.span);
|
||||
},
|
||||
BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => check(cx, right, 0, e.span, left.span),
|
||||
BinOpKind::Mul => {
|
||||
check(cx, left, 1, e.span, right.span);
|
||||
check(cx, right, 1, e.span, left.span);
|
||||
},
|
||||
BinOpKind::Div => check(cx, right, 1, e.span, left.span),
|
||||
BinOpKind::BitAnd => {
|
||||
check(cx, left, -1, e.span, right.span);
|
||||
check(cx, right, -1, e.span, left.span);
|
||||
},
|
||||
BinOpKind::Rem => check_remainder(cx, left, right, e.span, left.span),
|
||||
_ => (),
|
||||
if let ExprKind::Binary(cmp, left, right) = &expr.kind {
|
||||
if !is_allowed(cx, *cmp, left, right) {
|
||||
match cmp.node {
|
||||
BinOpKind::Add | BinOpKind::BitOr | BinOpKind::BitXor => {
|
||||
if reducible_to_right(cx, expr, right) {
|
||||
check(cx, left, 0, expr.span, right.span);
|
||||
}
|
||||
check(cx, right, 0, expr.span, left.span);
|
||||
},
|
||||
BinOpKind::Shl | BinOpKind::Shr | BinOpKind::Sub => {
|
||||
check(cx, right, 0, expr.span, left.span);
|
||||
},
|
||||
BinOpKind::Mul => {
|
||||
if reducible_to_right(cx, expr, right) {
|
||||
check(cx, left, 1, expr.span, right.span);
|
||||
}
|
||||
check(cx, right, 1, expr.span, left.span);
|
||||
},
|
||||
BinOpKind::Div => check(cx, right, 1, expr.span, left.span),
|
||||
BinOpKind::BitAnd => {
|
||||
if reducible_to_right(cx, expr, right) {
|
||||
check(cx, left, -1, expr.span, right.span);
|
||||
}
|
||||
check(cx, right, -1, expr.span, left.span);
|
||||
},
|
||||
BinOpKind::Rem => {
|
||||
// Don't call reducible_to_right because N % N is always reducible to 1
|
||||
check_remainder(cx, left, right, expr.span, left.span);
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if `left op ..right` can be actually reduced to `right`
|
||||
/// e.g. `0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }`
|
||||
/// cannot be reduced to `if b { 1 } else { 2 } + if b { 3 } else { 4 }`
|
||||
/// See #8724
|
||||
fn reducible_to_right(cx: &LateContext<'_>, binary: &Expr<'_>, right: &Expr<'_>) -> bool {
|
||||
if let ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Block(..) | ExprKind::Loop(..) = right.kind {
|
||||
is_toplevel_binary(cx, binary)
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
fn is_toplevel_binary(cx: &LateContext<'_>, must_be_binary: &Expr<'_>) -> bool {
|
||||
if let Some(parent) = get_parent_expr(cx, must_be_binary) && let ExprKind::Binary(..) = &parent.kind {
|
||||
false
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
|
||||
// This lint applies to integers
|
||||
!cx.typeck_results().expr_ty(left).peel_refs().is_integral()
|
||||
|
||||
@ -77,4 +77,34 @@ fn main() {
|
||||
(x + 1) % 3; // no error
|
||||
4 % 3; // no error
|
||||
4 % -3; // no error
|
||||
|
||||
// See #8724
|
||||
let a = 0;
|
||||
let b = true;
|
||||
0 + if b { 1 } else { 2 };
|
||||
0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }; // no error
|
||||
0 + match a { 0 => 10, _ => 20 };
|
||||
0 + match a { 0 => 10, _ => 20 } + match a { 0 => 30, _ => 40 }; // no error
|
||||
0 + if b { 1 } else { 2 } + match a { 0 => 30, _ => 40 }; // no error
|
||||
0 + match a { 0 => 10, _ => 20 } + if b { 3 } else { 4 }; // no error
|
||||
|
||||
0 + if b { 0 + 1 } else { 2 };
|
||||
0 + match a { 0 => 0 + 10, _ => 20 };
|
||||
0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 };
|
||||
|
||||
let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 };
|
||||
let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 };
|
||||
|
||||
0 + if b { 1 } else { 2 } + if b { 3 } else { 4 } + 0;
|
||||
|
||||
0 + { a } + 3; // no error
|
||||
0 + loop { let mut c = 0; if c == 10 { break c; } c += 1; } + { a * 2 }; // no error
|
||||
|
||||
fn f(_: i32) {
|
||||
todo!();
|
||||
}
|
||||
f(1 * a + { 8 * 5 });
|
||||
f(0 + if b { 1 } else { 2 } + 3); // no error
|
||||
const _: i32 = { 2 * 4 } + 0 + 3;
|
||||
const _: i32 = 0 + { 1 + 2 * 3 } + 3; // no error
|
||||
}
|
||||
|
||||
@ -108,5 +108,95 @@ error: the operation is ineffective. Consider reducing it to `1`
|
||||
LL | x + 1 % 3;
|
||||
| ^^^^^
|
||||
|
||||
error: aborting due to 18 previous errors
|
||||
error: the operation is ineffective. Consider reducing it to `if b { 1 } else { 2 }`
|
||||
--> $DIR/identity_op.rs:84:5
|
||||
|
|
||||
LL | 0 + if b { 1 } else { 2 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `match a { 0 => 10, _ => 20 }`
|
||||
--> $DIR/identity_op.rs:86:5
|
||||
|
|
||||
LL | 0 + match a { 0 => 10, _ => 20 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `if b { 0 + 1 } else { 2 }`
|
||||
--> $DIR/identity_op.rs:91:5
|
||||
|
|
||||
LL | 0 + if b { 0 + 1 } else { 2 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:91:16
|
||||
|
|
||||
LL | 0 + if b { 0 + 1 } else { 2 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `match a { 0 => 0 + 10, _ => 20 }`
|
||||
--> $DIR/identity_op.rs:92:5
|
||||
|
|
||||
LL | 0 + match a { 0 => 0 + 10, _ => 20 };
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `10`
|
||||
--> $DIR/identity_op.rs:92:25
|
||||
|
|
||||
LL | 0 + match a { 0 => 0 + 10, _ => 20 };
|
||||
| ^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:93:16
|
||||
|
|
||||
LL | 0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `30`
|
||||
--> $DIR/identity_op.rs:93:52
|
||||
|
|
||||
LL | 0 + if b { 0 + 1 } else { 2 } + match a { 0 => 0 + 30, _ => 40 };
|
||||
| ^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:95:20
|
||||
|
|
||||
LL | let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:95:52
|
||||
|
|
||||
LL | let _ = 0 + if 0 + 1 > 0 { 1 } else { 2 } + if 0 + 1 > 0 { 3 } else { 4 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:96:23
|
||||
|
|
||||
LL | let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `1`
|
||||
--> $DIR/identity_op.rs:96:58
|
||||
|
|
||||
LL | let _ = 0 + match 0 + 1 { 0 => 10, _ => 20 } + match 0 + 1 { 0 => 30, _ => 40 };
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `0 + if b { 1 } else { 2 } + if b { 3 } else { 4 }`
|
||||
--> $DIR/identity_op.rs:98:5
|
||||
|
|
||||
LL | 0 + if b { 1 } else { 2 } + if b { 3 } else { 4 } + 0;
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `a`
|
||||
--> $DIR/identity_op.rs:106:7
|
||||
|
|
||||
LL | f(1 * a + { 8 * 5 });
|
||||
| ^^^^^
|
||||
|
||||
error: the operation is ineffective. Consider reducing it to `{ 2 * 4 }`
|
||||
--> $DIR/identity_op.rs:108:20
|
||||
|
|
||||
LL | const _: i32 = { 2 * 4 } + 0 + 3;
|
||||
| ^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 33 previous errors
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user