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,14 @@
-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::ty::ty_from_hir_ty;
+use clippy_utils::{SpanlessEq, is_in_const_context, is_integer_literal};
 use rustc_errors::Applicability;
-use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
 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 +34,111 @@ declare_clippy_lint! {
  "manually reimplementing `is_power_of_two`"
 }
 
-declare_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
+pub struct ManualIsPowerOfTwo {
+ msrv: Msrv,
+}
 
-impl LateLintPass<'_> for ManualIsPowerOfTwo {
- fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
- let mut applicability = Applicability::MachineApplicable;
+impl_lint_pass!(ManualIsPowerOfTwo => [MANUAL_IS_POWER_OF_TWO]);
 
- if let ExprKind::Binary(bin_op, left, right) = expr.kind
- && 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);
- }
+impl ManualIsPowerOfTwo {
+ pub fn new(conf: &'static Conf) -> Self {
+ Self { msrv: conf.msrv }
+ }
 
- // 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);
- }
+ 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;
+ }
 
- // 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)
- {
- build_sugg(cx, expr, left1, &mut applicability);
- }
+ let mut applicability = Applicability::MachineApplicable;
+ let snippet = Sugg::hir_with_applicability(cx, receiver, "_", &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)
- {
- build_sugg(cx, expr, right1, &mut 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!("{}.is_power_of_two()", snippet.maybe_paren()),
+ 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)
+impl<'tcx> LateLintPass<'tcx> for ManualIsPowerOfTwo {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+ if !expr.span.from_expansion()
+ && let Some((lhs, rhs)) = unexpanded_binop_operands(expr, BinOpKind::Eq)
+ {
+ if let Some(a) = count_ones_receiver(cx, lhs)
+ && is_integer_literal(rhs, 1)
  {
- 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 let Some(a) = count_ones_receiver(cx, rhs)
+ && is_integer_literal(lhs, 1)
+ {
+ self.build_sugg(cx, expr, a);
+ } else if is_integer_literal(rhs, 0)
+ && let Some(a) = is_and_minus_one(cx, lhs)
+ {
+ 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()` or the argument of
+/// `<int-type>::count_ones(…)`.
+fn count_ones_receiver<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<&'tcx Expr<'tcx>> {
+ let (method, ty, receiver) = if let ExprKind::MethodCall(method_name, receiver, [], _) = expr.kind {
+ (method_name, cx.typeck_results().expr_ty_adjusted(receiver), receiver)
+ } else if let ExprKind::Call(func, [arg]) = expr.kind
+ && let ExprKind::Path(QPath::TypeRelative(ty, func_name)) = func.kind
+ {
+ (func_name, ty_from_hir_ty(cx, ty), arg)
+ } else {
+ return None;
+ };
+ (method.ident.as_str() == "count_ones" && matches!(ty.kind(), ty::Uint(_))).then_some(receiver)
 }
 
-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 Some((lhs, rhs)) = unexpanded_binop_operands(smaller, 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>> {
+ let (lhs, rhs) = unexpanded_binop_operands(expr, BinOpKind::BitAnd)?;
+ is_one_less(cx, lhs, rhs).or_else(|| is_one_less(cx, rhs, lhs))
+}
+
+/// Return the operands of the `expr` binary operation if the operator is `op` and none of the
+/// operands come from expansion.
+fn unexpanded_binop_operands<'hir>(expr: &Expr<'hir>, op: BinOpKind) -> Option<(&'hir Expr<'hir>, &'hir Expr<'hir>)> {
+ if let ExprKind::Binary(binop, lhs, rhs) = expr.kind
+ && binop.node == op
+ && !lhs.span.from_expansion()
+ && !rhs.span.from_expansion()
+ {
+ Some((lhs, rhs))
+ } 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;
@@ -7,6 +20,8 @@ fn main() {
  //~^ manual_is_power_of_two
  let _ = a.is_power_of_two();
  //~^ manual_is_power_of_two
+ let _ = a.is_power_of_two();
+ //~^ manual_is_power_of_two
 
  // Test different orders of expression
  let _ = a.is_power_of_two();
@@ -23,4 +38,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,10 +1,25 @@
 #![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;
 
  let _ = a.count_ones() == 1;
  //~^ manual_is_power_of_two
+ let _ = u64::count_ones(a) == 1;
+ //~^ manual_is_power_of_two
  let _ = a & (a - 1) == 0;
  //~^ manual_is_power_of_two
 
@@ -23,4 +38,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
 }