Various fixes for manual_is_power_of_two

book/src/lint_configuration.md

@@ -804,6 +804,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio
 * [`manual_flatten`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten)
 * [`manual_hash_one`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one)
 * [`manual_is_ascii_check`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check)
+* [`manual_is_power_of_two`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_power_of_two)
 * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else)
 * [`manual_midpoint`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint)
 * [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive)

clippy_config/src/conf.rs

@@ -721,6 +721,7 @@ define_Conf! {
         manual_flatten,
         manual_hash_one,
         manual_is_ascii_check,
+        manual_is_power_of_two,
         manual_let_else,
         manual_midpoint,
         manual_non_exhaustive,

clippy_lints/src/lib.rs

@@ -969,7 +969,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(zombie_processes::ZombieProcesses));
     store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
     store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
-    store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
+    store.register_late_pass(move |_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo::new(conf)));
     store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
     store.register_late_pass(|_| Box::new(literal_string_with_formatting_args::LiteralStringWithFormattingArg));
     store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf)));

clippy_lints/src/manual_is_power_of_two.rs

@@ -1,13 +1,13 @@
-use clippy_utils::SpanlessEq;
+use clippy_config::Conf;
 use clippy_utils::diagnostics::span_lint_and_sugg;
-use clippy_utils::source::snippet_with_applicability;
-use rustc_ast::LitKind;
-use rustc_data_structures::packed::Pu128;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{SpanlessEq, is_in_const_context, is_integer_literal};
 use rustc_errors::Applicability;
 use rustc_hir::{BinOpKind, Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::Uint;
-use rustc_session::declare_lint_pass;
+use rustc_middle::ty;
+use rustc_session::impl_lint_pass;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -33,112 +33,107 @@ declare_clippy_lint! {
     "manually reimplementing `is_power_of_two`"
 }
 
-declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
+pub struct ManualIsPowerOfTwo {
+    msrv: Msrv,
+}
+
+impl_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
+
+impl ManualIsPowerOfTwo {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self { msrv: conf.msrv }
+    }
+
+    fn build_sugg(&self, cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>) {
+        if is_in_const_context(cx) && !self.msrv.meets(cx, msrvs::CONST_IS_POWER_OF_TWO) {
+            return;
+        }
 
-impl LateLintPass<'_> for ManualIsPowerOfTwo {
-    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
         let mut applicability = Applicability::MachineApplicable;
+        let snippet = Sugg::hir_with_applicability(cx, receiver, "_", &mut applicability);
 
-        if let ExprKind::Binary(bin_op, left, right) = expr.kind
+        span_lint_and_sugg(
+            cx,
+            MANUAL_IS_POWER_OF_TWO,
+            expr.span,
+            "manually reimplementing `is_power_of_two`",
+            "consider using `.is_power_of_two()`",
+            format!("{}.is_power_of_two()", snippet.maybe_paren()),
+            applicability,
+        );
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualIsPowerOfTwo {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if !expr.span.from_expansion()
+            && let ExprKind::Binary(bin_op, lhs, rhs) = expr.kind
+            && !lhs.span.from_expansion()
+            && !rhs.span.from_expansion()
             && bin_op.node == BinOpKind::Eq
         {
-            // a.count_ones() == 1
-            if let ExprKind::MethodCall(method_name, receiver, [], _) = left.kind
-                && method_name.ident.as_str() == "count_ones"
-                && let &Uint(_) = cx.typeck_results().expr_ty(receiver).kind()
-                && check_lit(right, 1)
-            {
-                build_sugg(cx, expr, receiver, &mut applicability);
-            }
-
-            // 1 == a.count_ones()
-            if let ExprKind::MethodCall(method_name, receiver, [], _) = right.kind
-                && method_name.ident.as_str() == "count_ones"
-                && let &Uint(_) = cx.typeck_results().expr_ty(receiver).kind()
-                && check_lit(left, 1)
-            {
-                build_sugg(cx, expr, receiver, &mut applicability);
-            }
-
-            // a & (a - 1) == 0
-            if let ExprKind::Binary(op1, left1, right1) = left.kind
-                && op1.node == BinOpKind::BitAnd
-                && let ExprKind::Binary(op2, left2, right2) = right1.kind
-                && op2.node == BinOpKind::Sub
-                && check_eq_expr(cx, left1, left2)
-                && let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
-                && check_lit(right2, 1)
-                && check_lit(right, 0)
+            if let Some(a) = count_ones_receiver(cx, lhs)
+                && is_integer_literal(rhs, 1)
             {
-                build_sugg(cx, expr, left1, &mut applicability);
-            }
-
-            // (a - 1) & a == 0;
-            if let ExprKind::Binary(op1, left1, right1) = left.kind
-                && op1.node == BinOpKind::BitAnd
-                && let ExprKind::Binary(op2, left2, right2) = left1.kind
-                && op2.node == BinOpKind::Sub
-                && check_eq_expr(cx, right1, left2)
-                && let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
-                && check_lit(right2, 1)
-                && check_lit(right, 0)
+                self.build_sugg(cx, expr, a);
+            } else if let Some(a) = count_ones_receiver(cx, rhs)
+                && is_integer_literal(lhs, 1)
             {
-                build_sugg(cx, expr, right1, &mut applicability);
-            }
-
-            // 0 == a & (a - 1);
-            if let ExprKind::Binary(op1, left1, right1) = right.kind
-                && op1.node == BinOpKind::BitAnd
-                && let ExprKind::Binary(op2, left2, right2) = right1.kind
-                && op2.node == BinOpKind::Sub
-                && check_eq_expr(cx, left1, left2)
-                && let &Uint(_) = cx.typeck_results().expr_ty(left1).kind()
-                && check_lit(right2, 1)
-                && check_lit(left, 0)
+                self.build_sugg(cx, expr, a);
+            } else if is_integer_literal(rhs, 0)
+                && let Some(a) = is_and_minus_one(cx, lhs)
             {
-                build_sugg(cx, expr, left1, &mut applicability);
-            }
-
-            // 0 == (a - 1) & a
-            if let ExprKind::Binary(op1, left1, right1) = right.kind
-                && op1.node == BinOpKind::BitAnd
-                && let ExprKind::Binary(op2, left2, right2) = left1.kind
-                && op2.node == BinOpKind::Sub
-                && check_eq_expr(cx, right1, left2)
-                && let &Uint(_) = cx.typeck_results().expr_ty(right1).kind()
-                && check_lit(right2, 1)
-                && check_lit(left, 0)
+                self.build_sugg(cx, expr, a);
+            } else if is_integer_literal(lhs, 0)
+                && let Some(a) = is_and_minus_one(cx, rhs)
             {
-                build_sugg(cx, expr, right1, &mut applicability);
+                self.build_sugg(cx, expr, a);
             }
         }
     }
 }
 
-fn build_sugg(cx: &LateContext<'_>, expr: &Expr<'_>, receiver: &Expr<'_>, applicability: &mut Applicability) {
-    let snippet = snippet_with_applicability(cx, receiver.span, "..", applicability);
-
-    span_lint_and_sugg(
-        cx,
-        MANUAL_IS_POWER_OF_TWO,
-        expr.span,
-        "manually reimplementing `is_power_of_two`",
-        "consider using `.is_power_of_two()`",
-        format!("{snippet}.is_power_of_two()"),
-        *applicability,
-    );
+/// Return the unsigned integer receiver of `.count_ones()`
+fn count_ones_receiver<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if let ExprKind::MethodCall(method_name, receiver, [], _) = expr.kind
+        && method_name.ident.as_str() == "count_ones"
+        && matches!(cx.typeck_results().expr_ty_adjusted(receiver).kind(), ty::Uint(_))
+    {
+        Some(receiver)
+    } else {
+        None
+    }
 }
 
-fn check_lit(expr: &Expr<'_>, expected_num: u128) -> bool {
-    if let ExprKind::Lit(lit) = expr.kind
-        && let LitKind::Int(Pu128(num), _) = lit.node
-        && num == expected_num
+/// Return `greater` if `smaller == greater - 1`
+fn is_one_less<'tcx>(
+    cx: &LateContext<'tcx>,
+    greater: &'tcx Expr<'tcx>,
+    smaller: &Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    if let ExprKind::Binary(op, lhs, rhs) = smaller.kind
+        && !lhs.span.from_expansion()
+        && !rhs.span.from_expansion()
+        && op.node == BinOpKind::Sub
+        && SpanlessEq::new(cx).eq_expr(greater, lhs)
+        && is_integer_literal(rhs, 1)
+        && matches!(cx.typeck_results().expr_ty_adjusted(greater).kind(), ty::Uint(_))
     {
-        return true;
+        Some(greater)
+    } else {
+        None
     }
-    false
 }
 
-fn check_eq_expr(cx: &LateContext<'_>, lhs: &Expr<'_>, rhs: &Expr<'_>) -> bool {
-    SpanlessEq::new(cx).eq_expr(lhs, rhs)
+/// Return `v` if `expr` is `v & (v - 1)` or `(v - 1) & v`
+fn is_and_minus_one<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+    if let ExprKind::Binary(op, lhs, rhs) = expr.kind
+        && !lhs.span.from_expansion()
+        && !rhs.span.from_expansion()
+        && op.node == BinOpKind::BitAnd
+    {
+        is_one_less(cx, lhs, rhs).or_else(|| is_one_less(cx, rhs, lhs))
+    } else {
+        None
+    }
 }

clippy_utils/src/msrvs.rs

@@ -63,6 +63,7 @@ msrv_aliases! {
     1,35,0 { OPTION_COPIED, RANGE_CONTAINS }
     1,34,0 { TRY_FROM }
     1,33,0 { UNDERSCORE_IMPORTS }
+    1,32,0 { CONST_IS_POWER_OF_TWO }
     1,31,0 { OPTION_REPLACE }
     1,30,0 { ITERATOR_FIND_MAP, TOOL_ATTRIBUTES }
     1,29,0 { ITER_FLATTEN }

tests/ui/manual_is_power_of_two.fixed

@@ -1,4 +1,17 @@
 #![warn(clippy::manual_is_power_of_two)]
+#![allow(clippy::precedence)]
+
+macro_rules! binop {
+    ($a: expr, equal, $b: expr) => {
+        $a == $b
+    };
+    ($a: expr, and, $b: expr) => {
+        $a & $b
+    };
+    ($a: expr, minus, $b: expr) => {
+        $a - $b
+    };
+}
 
 fn main() {
     let a = 16_u64;
@@ -23,4 +36,23 @@ fn main() {
     // is_power_of_two only works for unsigned integers
     let _ = b.count_ones() == 1;
     let _ = b & (b - 1) == 0;
+
+    let i: i32 = 3;
+    let _ = (i as u32).is_power_of_two();
+    //~^ manual_is_power_of_two
+
+    let _ = binop!(a.count_ones(), equal, 1);
+    let _ = binop!(a, and, a - 1) == 0;
+    let _ = a & binop!(a, minus, 1) == 0;
+}
+
+#[clippy::msrv = "1.31"]
+const fn low_msrv(a: u32) -> bool {
+    a & (a - 1) == 0
+}
+
+#[clippy::msrv = "1.32"]
+const fn high_msrv(a: u32) -> bool {
+    a.is_power_of_two()
+    //~^ manual_is_power_of_two
 }

tests/ui/manual_is_power_of_two.rs

@@ -1,4 +1,17 @@
 #![warn(clippy::manual_is_power_of_two)]
+#![allow(clippy::precedence)]
+
+macro_rules! binop {
+    ($a: expr, equal, $b: expr) => {
+        $a == $b
+    };
+    ($a: expr, and, $b: expr) => {
+        $a & $b
+    };
+    ($a: expr, minus, $b: expr) => {
+        $a - $b
+    };
+}
 
 fn main() {
     let a = 16_u64;
@@ -23,4 +36,23 @@ fn main() {
     // is_power_of_two only works for unsigned integers
     let _ = b.count_ones() == 1;
     let _ = b & (b - 1) == 0;
+
+    let i: i32 = 3;
+    let _ = i as u32 & (i as u32 - 1) == 0;
+    //~^ manual_is_power_of_two
+
+    let _ = binop!(a.count_ones(), equal, 1);
+    let _ = binop!(a, and, a - 1) == 0;
+    let _ = a & binop!(a, minus, 1) == 0;
+}
+
+#[clippy::msrv = "1.31"]
+const fn low_msrv(a: u32) -> bool {
+    a & (a - 1) == 0
+}
+
+#[clippy::msrv = "1.32"]
+const fn high_msrv(a: u32) -> bool {
+    a & (a - 1) == 0
+    //~^ manual_is_power_of_two
 }

tests/ui/manual_is_power_of_two.stderr

@@ -1,5 +1,5 @@
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:6:13
+  --> tests/ui/manual_is_power_of_two.rs:19:13
    |
 LL |     let _ = a.count_ones() == 1;
    |             ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
@@ -8,34 +8,46 @@ LL |     let _ = a.count_ones() == 1;
    = help: to override `-D warnings` add `#[allow(clippy::manual_is_power_of_two)]`
 
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:8:13
+  --> tests/ui/manual_is_power_of_two.rs:21:13
    |
 LL |     let _ = a & (a - 1) == 0;
    |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:12:13
+  --> tests/ui/manual_is_power_of_two.rs:25:13
    |
 LL |     let _ = 1 == a.count_ones();
    |             ^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:14:13
+  --> tests/ui/manual_is_power_of_two.rs:27:13
    |
 LL |     let _ = (a - 1) & a == 0;
    |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:16:13
+  --> tests/ui/manual_is_power_of_two.rs:29:13
    |
 LL |     let _ = 0 == a & (a - 1);
    |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
 error: manually reimplementing `is_power_of_two`
-  --> tests/ui/manual_is_power_of_two.rs:18:13
+  --> tests/ui/manual_is_power_of_two.rs:31:13
    |
 LL |     let _ = 0 == (a - 1) & a;
    |             ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
 
-error: aborting due to 6 previous errors
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:41:13
+   |
+LL |     let _ = i as u32 & (i as u32 - 1) == 0;
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `(i as u32).is_power_of_two()`
+
+error: manually reimplementing `is_power_of_two`
+  --> tests/ui/manual_is_power_of_two.rs:56:5
+   |
+LL |     a & (a - 1) == 0
+   |     ^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `a.is_power_of_two()`
+
+error: aborting due to 8 previous errors