Split `clippy::manual_is_power_of_two` into two lints to handle `x & (x - 1) == 0` better

[daira]

What it does

The existing clippy::manual_is_power_of_two lint, which triggers on expressions of the form either x.count_ones() == 1 or x & (x - 1) == 0 where x is of unsigned integer type, was moved from default to pedantic in #13553. The rationale stated in #13547 was that x.count_ones() == 1 might be a clearer way of expressing the intent if x represents a bit flag, for example.

The suggestion is to:

• Change clippy::manual_is_power_of_two to trigger only on x.count_ones() == 1.
• Add, say clippy::bithack_is_power_of_two that triggers only on x & (x - 1) == 0 and similar. The message should make clear that another reason not to use this form is that it is incorrect for x == 0 (or for x <= 0 if x is of a signed type).

The reason to split into two lints, rather than just have clippy::manual_is_power_of_two produce different lint messages, is that it is quite plausible that users would want to configure them differently. For example, the reporter of #13547 would only have needed to suppress clippy::manual_is_power_of_two.

Also, clippy::manual_is_power_of_two is currently marked as MachineApplicable ("The suggestion is definitely what the user intended, or maintains the exact meaning of the code. This suggestion should be automatically applied."), but it does not maintain the exact meaning of the code when applied to x & (x - 1) == 0 for x == 0: in debug mode it removes a panic and in release mode it changes the result for that case. This is a bug.

Note that in Rust, if x is the minimum value of its type then x - 1 is unspecified behaviour; it may panic in debug mode. So the correct expression would have to be x > 0 && x & (x - 1) == 0 or equivalent. If the code actually uses this expression then the lint message might need to be different.

If x is of a signed type, then x.is_power_of_two() doesn't exist. A programmer might still try to use x > 0 && x & (x - 1) == 0 there — which is correct, if the intent is only to check for positive powers of two. For bonus points suggest something like uN::try_from(x).is_ok_and(|u| u.is_power_of_two()) (where uN is the appropriate unsigned integer type) in that case.

Advantages

It's clearer and handles x <= 0 correctly.

It covers the case of signed integer types.

It can be correctly marked as MachineApplicable provided that it only performs fixes as noted in the Examples below.

The existing clippy::manual_is_power_of_two lint had false positives that caused it to be moved to pedantic. clippy::bithack_is_power_of_two could potentially be default-enabled since it does not have those false positives.

Drawbacks

There might be reason to use x & (x - 1) == 0 in code that needs to operate on secret data in constant time and where x is guaranteed to be positive. (Either x.count_ones() or x.is_power_of_two() might be non-constant-time, especially on architectures that have no "popcount" instruction.) That probably doesn't happen very often and in that case I think it would be reasonable to suppress the lint.

If a user had explicitly disabled clippy::manual_is_power_of_two during the period when it was default-enabled, and now need to disable clippy::bithack_is_power_of_two, they might be irritated. This seems like a relatively minor issue.

Examples

let x: u32 = ...;
x & (x - 1) == 0

warning: reimplementing is_power_of_two using a bit hack

x & (x - 1) == 0
^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `x.is_power_of_two()`

= help: Note that x - 1 is unspecified when x == 0. It will panic in debug mode, or give the wrong answer in release mode if it was intended to check for a power of two.
= help: If the intent was to yield true for x == 0, use x == 0 || x.is_power_of_two().
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bithack_is_power_of_two

This case should not perform an automatic fix since it changes the semantics.

---

let x: u32 = ...;
x > 0 && x & (x - 1) == 0

warning: reimplementing is_power_of_two using a bit hack

x > 0 && x & (x - 1) == 0
^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.is_power_of_two()`: `x.is_power_of_two()`

= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bithack_is_power_of_two

This can and should perform an automatic fix with --fix.

---

let x: i32 = ...;
x & (x - 1) == 0

warning: reimplementing is_power_of_two using a bit hack

x & (x - 1) == 0
^^^^^^^^^^^^^^^^ help: consider converting to u32 and using `.is_power_of_two()`:
`u32::try_from(x).is_ok_and(|u| u.is_power_of_two())`

= help: Note that x - 1 is unspecified when x == i32::MIN. It will panic in debug mode. Also, this code will give the wrong answer for x <= 0 if it was intended to check for a power of two.
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bithack_is_power_of_two

This case should not perform an automatic fix since it changes the semantics.

---

let x: i32 = ...;
x > 0 && x & (x - 1) == 0

warning: reimplementing is_power_of_two using a bit hack

x > 0 && x & (x - 1) == 0
^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider consider converting to u32 and using `.is_power_of_two()`:
`u32::try_from(x).is_ok_and(|u| u.is_power_of_two())`

= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bithack_is_power_of_two

This could perform an automatic fix; it's a matter of opinion.