Skip to content

Commit

Permalink
Auto merge of #8183 - alex-ozdemir:limit-ident, r=camsteffen
Browse files Browse the repository at this point in the history
Limit the ``[`identity_op`]`` lint to integral operands.

changelog: limit ``[`identity_op`]`` to integral operands

In the ``[`identity_op`]`` lint, if the operands are non-integers, then the lint is likely
wrong.
  • Loading branch information
bors committed Dec 28, 2021
2 parents a139949 + ee6d5c5 commit 16ef044
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 18 deletions.
15 changes: 9 additions & 6 deletions clippy_lints/src/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,18 @@ impl<'tcx> LateLintPass<'tcx> for IdentityOp {
}

fn is_allowed(cx: &LateContext<'_>, cmp: BinOp, left: &Expr<'_>, right: &Expr<'_>) -> bool {
// `1 << 0` is a common pattern in bit manipulation code
cmp.node == BinOpKind::Shl
&& constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
&& constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1))
// This lint applies to integers
!cx.typeck_results().expr_ty(left).peel_refs().is_integral()
|| !cx.typeck_results().expr_ty(right).peel_refs().is_integral()
// `1 << 0` is a common pattern in bit manipulation code
|| (cmp.node == BinOpKind::Shl
&& constant_simple(cx, cx.typeck_results(), right) == Some(Constant::Int(0))
&& constant_simple(cx, cx.typeck_results(), left) == Some(Constant::Int(1)))
}

fn check(cx: &LateContext<'_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) {
if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e) {
let check = match *cx.typeck_results().expr_ty(e).kind() {
if let Some(Constant::Int(v)) = constant_simple(cx, cx.typeck_results(), e).map(Constant::peel_refs) {
let check = match *cx.typeck_results().expr_ty(e).peel_refs().kind() {
ty::Int(ity) => unsext(cx.tcx, -1_i128, ity),
ty::Uint(uty) => clip(cx.tcx, !0, uty),
_ => return,
Expand Down
8 changes: 8 additions & 0 deletions clippy_utils/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ impl Constant {
None
}
}

#[must_use]
pub fn peel_refs(mut self) -> Self {
while let Constant::Ref(r) = self {
self = *r;
}
self
}
}

/// Parses a `LitKind` to a `Constant`.
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,20 @@ const ONE: i64 = 1;
const NEG_ONE: i64 = -1;
const ZERO: i64 = 0;

struct A(String);

impl std::ops::Shl<i32> for A {
type Output = A;
fn shl(mut self, other: i32) -> Self {
self.0.push_str(&format!("{}", other));
self
}
}
#[allow(
clippy::eq_op,
clippy::no_effect,
clippy::unnecessary_operation,
clippy::op_ref,
clippy::double_parens
)]
#[warn(clippy::identity_op)]
Expand Down Expand Up @@ -38,4 +48,9 @@ fn main() {
42 << 0;
1 >> 0;
42 >> 0;
&x >> 0;
x >> &0;

let mut a = A("".into());
let b = a << 0; // no error: non-integer
}
36 changes: 24 additions & 12 deletions tests/ui/identity_op.stderr
Original file line number Diff line number Diff line change
@@ -1,70 +1,82 @@
error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:16:5
--> $DIR/identity_op.rs:26:5
|
LL | x + 0;
| ^^^^^
|
= note: `-D clippy::identity-op` implied by `-D warnings`

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:17:5
--> $DIR/identity_op.rs:27:5
|
LL | x + (1 - 1);
| ^^^^^^^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:19:5
--> $DIR/identity_op.rs:29:5
|
LL | 0 + x;
| ^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:22:5
--> $DIR/identity_op.rs:32:5
|
LL | x | (0);
| ^^^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:25:5
--> $DIR/identity_op.rs:35:5
|
LL | x * 1;
| ^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:26:5
--> $DIR/identity_op.rs:36:5
|
LL | 1 * x;
| ^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:32:5
--> $DIR/identity_op.rs:42:5
|
LL | -1 & x;
| ^^^^^^

error: the operation is ineffective. Consider reducing it to `u`
--> $DIR/identity_op.rs:35:5
--> $DIR/identity_op.rs:45:5
|
LL | u & 255;
| ^^^^^^^

error: the operation is ineffective. Consider reducing it to `42`
--> $DIR/identity_op.rs:38:5
--> $DIR/identity_op.rs:48:5
|
LL | 42 << 0;
| ^^^^^^^

error: the operation is ineffective. Consider reducing it to `1`
--> $DIR/identity_op.rs:39:5
--> $DIR/identity_op.rs:49:5
|
LL | 1 >> 0;
| ^^^^^^

error: the operation is ineffective. Consider reducing it to `42`
--> $DIR/identity_op.rs:40:5
--> $DIR/identity_op.rs:50:5
|
LL | 42 >> 0;
| ^^^^^^^

error: aborting due to 11 previous errors
error: the operation is ineffective. Consider reducing it to `&x`
--> $DIR/identity_op.rs:51:5
|
LL | &x >> 0;
| ^^^^^^^

error: the operation is ineffective. Consider reducing it to `x`
--> $DIR/identity_op.rs:52:5
|
LL | x >> &0;
| ^^^^^^^

error: aborting due to 13 previous errors

0 comments on commit 16ef044

Please sign in to comment.