Skip to content

Commit

Permalink
Auto merge of #5602 - ebroto:issue_3430, r=phansch
Browse files Browse the repository at this point in the history
identity_op: allow `1 << 0`

I went for accepting `1 << 0` verbatim instead of something more general as it seems to be what everyone in the issue thread needed.

changelog: identity_op: allow `1 << 0` as it's a common pattern in bit manipulation code.

Fixes #3430
  • Loading branch information
bors committed May 16, 2020
2 parents cac9ad0 + fc8ab09 commit 53a9805
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 3 deletions.
22 changes: 20 additions & 2 deletions clippy_lints/src/identity_op.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_hir::{BinOpKind, Expr, ExprKind};
use if_chain::if_chain;
use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -32,7 +33,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for IdentityOp {
if e.span.from_expansion() {
return;
}
if let ExprKind::Binary(ref cmp, ref left, ref right) = e.kind {
if let ExprKind::Binary(cmp, ref left, ref 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);
Expand All @@ -54,6 +58,20 @@ impl<'a, 'tcx> LateLintPass<'a, '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
if_chain! {
if let BinOpKind::Shl = cmp.node;
if let Some(Constant::Int(0)) = constant_simple(cx, cx.tables, right);
if let Some(Constant::Int(1)) = constant_simple(cx, cx.tables, left);
then {
return true;
}
}

false
}

#[allow(clippy::cast_possible_wrap)]
fn check(cx: &LateContext<'_, '_>, e: &Expr<'_>, m: i8, span: Span, arg: Span) {
if let Some(Constant::Int(v)) = constant_simple(cx, cx.tables, e) {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/identity_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,9 @@ fn main() {

let u: u8 = 0;
u & 255;

1 << 0; // no error, this case is allowed, see issue 3430
42 << 0;
1 >> 0;
42 >> 0;
}
20 changes: 19 additions & 1 deletion tests/ui/identity_op.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,23 @@ error: the operation is ineffective. Consider reducing it to `u`
LL | u & 255;
| ^^^^^^^

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

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

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

error: aborting due to 11 previous errors

0 comments on commit 53a9805

Please sign in to comment.