Skip to content

Commit 1cab0b4

Browse files
committed
Do not lint result from macro expansion
If parts of the expression comes from macro expansion, it may match an expression equivalent to `is_power_of_two()` by chance only.
1 parent 89385c1 commit 1cab0b4

File tree

4 files changed

+56
-14
lines changed

4 files changed

+56
-14
lines changed

clippy_lints/src/manual_is_power_of_two.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
3535

3636
impl<'tcx> LateLintPass<'tcx> for ManualIsPowerOfTwo {
3737
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
38-
if let ExprKind::Binary(bin_op, lhs, rhs) = expr.kind
39-
&& bin_op.node == BinOpKind::Eq
38+
if !expr.span.from_expansion()
39+
&& let Some((lhs, rhs)) = unexpanded_binop_operands(expr, BinOpKind::Eq)
4040
{
4141
if let Some(a) = count_ones_receiver(cx, lhs)
4242
&& is_integer_literal(rhs, 1)
@@ -92,8 +92,7 @@ fn is_one_less<'tcx>(
9292
greater: &'tcx Expr<'tcx>,
9393
smaller: &Expr<'tcx>,
9494
) -> Option<&'tcx Expr<'tcx>> {
95-
if let ExprKind::Binary(op, lhs, rhs) = smaller.kind
96-
&& op.node == BinOpKind::Sub
95+
if let Some((lhs, rhs)) = unexpanded_binop_operands(smaller, BinOpKind::Sub)
9796
&& SpanlessEq::new(cx).eq_expr(greater, lhs)
9897
&& is_integer_literal(rhs, 1)
9998
&& matches!(cx.typeck_results().expr_ty_adjusted(greater).kind(), ty::Uint(_))
@@ -106,10 +105,19 @@ fn is_one_less<'tcx>(
106105

107106
/// Return `v` if `expr` is `v & (v - 1)` or `(v - 1) & v`
108107
fn is_and_minus_one<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
109-
if let ExprKind::Binary(op, lhs, rhs) = expr.kind
110-
&& op.node == BinOpKind::BitAnd
108+
let (lhs, rhs) = unexpanded_binop_operands(expr, BinOpKind::BitAnd)?;
109+
is_one_less(cx, lhs, rhs).or_else(|| is_one_less(cx, rhs, lhs))
110+
}
111+
112+
/// Return the operands of the `expr` binary operation if the operator is `op` and none of the
113+
/// operands come from expansion.
114+
fn unexpanded_binop_operands<'hir>(expr: &Expr<'hir>, op: BinOpKind) -> Option<(&'hir Expr<'hir>, &'hir Expr<'hir>)> {
115+
if let ExprKind::Binary(binop, lhs, rhs) = expr.kind
116+
&& binop.node == op
117+
&& !lhs.span.from_expansion()
118+
&& !rhs.span.from_expansion()
111119
{
112-
is_one_less(cx, lhs, rhs).or_else(|| is_one_less(cx, rhs, lhs))
120+
Some((lhs, rhs))
113121
} else {
114122
None
115123
}

tests/ui/manual_is_power_of_two.fixed

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
#![warn(clippy::manual_is_power_of_two)]
2+
#![allow(clippy::precedence)]
3+
4+
macro_rules! binop {
5+
($a: expr, equal, $b: expr) => {
6+
$a == $b
7+
};
8+
($a: expr, and, $b: expr) => {
9+
$a & $b
10+
};
11+
($a: expr, minus, $b: expr) => {
12+
$a - $b
13+
};
14+
}
215

316
fn main() {
417
let a = 16_u64;
@@ -27,4 +40,8 @@ fn main() {
2740
let i: i32 = 3;
2841
let _ = (i as u32).is_power_of_two();
2942
//~^ manual_is_power_of_two
43+
44+
let _ = binop!(a.count_ones(), equal, 1);
45+
let _ = binop!(a, and, a - 1) == 0;
46+
let _ = a & binop!(a, minus, 1) == 0;
3047
}

tests/ui/manual_is_power_of_two.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,17 @@
11
#![warn(clippy::manual_is_power_of_two)]
2+
#![allow(clippy::precedence)]
3+
4+
macro_rules! binop {
5+
($a: expr, equal, $b: expr) => {
6+
$a == $b
7+
};
8+
($a: expr, and, $b: expr) => {
9+
$a & $b
10+
};
11+
($a: expr, minus, $b: expr) => {
12+
$a - $b
13+
};
14+
}
215

316
fn main() {
417
let a = 16_u64;
@@ -27,4 +40,8 @@ fn main() {
2740
let i: i32 = 3;
2841
let _ = i as u32 & (i as u32 - 1) == 0;
2942
//~^ manual_is_power_of_two
43+
44+
let _ = binop!(a.count_ones(), equal, 1);
45+
let _ = binop!(a, and, a - 1) == 0;
46+
let _ = a & binop!(a, minus, 1) == 0;
3047
}

tests/ui/manual_is_power_of_two.stderr

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: manually reimplementing `is_power_of_two`
2-
--> tests/ui/manual_is_power_of_two.rs:6:13
2+
--> tests/ui/manual_is_power_of_two.rs:19:13
33
|
44
LL | let _ = a.count_ones() == 1;
55
| ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
@@ -8,37 +8,37 @@ LL | let _ = a.count_ones() == 1;
88
= help: to override `-D warnings` add `#[allow(clippy::manual_is_power_of_two)]`
99

1010
error: manually reimplementing `is_power_of_two`
11-
--> tests/ui/manual_is_power_of_two.rs:8:13
11+
--> tests/ui/manual_is_power_of_two.rs:21:13
1212
|
1313
LL | let _ = a & (a - 1) == 0;
1414
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
1515

1616
error: manually reimplementing `is_power_of_two`
17-
--> tests/ui/manual_is_power_of_two.rs:12:13
17+
--> tests/ui/manual_is_power_of_two.rs:25:13
1818
|
1919
LL | let _ = 1 == a.count_ones();
2020
| ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
2121

2222
error: manually reimplementing `is_power_of_two`
23-
--> tests/ui/manual_is_power_of_two.rs:14:13
23+
--> tests/ui/manual_is_power_of_two.rs:27:13
2424
|
2525
LL | let _ = (a - 1) & a == 0;
2626
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
2727

2828
error: manually reimplementing `is_power_of_two`
29-
--> tests/ui/manual_is_power_of_two.rs:16:13
29+
--> tests/ui/manual_is_power_of_two.rs:29:13
3030
|
3131
LL | let _ = 0 == a & (a - 1);
3232
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
3333

3434
error: manually reimplementing `is_power_of_two`
35-
--> tests/ui/manual_is_power_of_two.rs:18:13
35+
--> tests/ui/manual_is_power_of_two.rs:31:13
3636
|
3737
LL | let _ = 0 == (a - 1) & a;
3838
| ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
3939

4040
error: manually reimplementing `is_power_of_two`
41-
--> tests/ui/manual_is_power_of_two.rs:28:13
41+
--> tests/ui/manual_is_power_of_two.rs:41:13
4242
|
4343
LL | let _ = i as u32 & (i as u32 - 1) == 0;
4444
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `(i as u32).is_power_of_two()`

0 commit comments

Comments
 (0)