Consider the following:
pub const fn WTERMSIG(s: c::c_int) -> c::c_int {
s & 0x7f
}
pub const fn WIFEXITED(s: c::c_int) -> bool {
s & 0x7f == 0 // <--- try: `s.trailing_zeros() >= 7`
}
The documentation for verbose_bit_mask has the following example:
x.trailing_zeros() > 4is much clearer thanx & 15 == 0
But this statement is wrong in the general case. Code such as x & 15 is often accompanied by other bit-fiddling to extract certain parts of the integer such as in the example above. Using trailing_zeros in one place and & 15 in the other makes the code as a whole much less clear.
verbose_bit_mask should be disabled by default.
cc @oli-obk
While I can imagine that there are situations where the lint would make the code less readable, I don't think the above example is a motivating example for disabling the lint. While the fix for the situation is not the one suggested by the lint, using
pub const fn WIFEXITED(s: c::c_int) -> bool {
WTERMSIG(s) == 0
}
has less duplication and is much clearer imo. The lint succeeded in highlighting this situation that deserved improvement, so to me it has done its job.
The lint succeeded in highlighting this situation that deserved improvement, so to me it has done its job.
fn is_type_a(x: u32) -> bool {
x & 0xff00 == 0
}
fn is_type_b(x: u32) -> bool {
x & 0x00ff == 0 // try: `x.trailing_zeros() >= 8`
}
Which situation is clippy highlighting in this example?
@oli-obk The reason I tagged you was so that you could explain why you added this lint in the first place. The commit message contains no information. I'm not aware of ever seeing any piece of code where someone used masking where using a dedicated function for counting trailing zeros would have been clearer.
This new example shows where the current lint fails indeed. While I personally would like it extended to lint x & 0xff00 == 0 and suggest x.leading_zeros() >= 8, too, I agree that this is a very opinionated lint and be moved to pedantic
Most helpful comment
This new example shows where the current lint fails indeed. While I personally would like it extended to lint
x & 0xff00 == 0and suggestx.leading_zeros() >= 8, too, I agree that this is a very opinionated lint and be moved topedantic