-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
verbose_bit_mask is questionable #5633
Comments
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. |
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 |
Consider the following:
The documentation for verbose_bit_mask has the following example:
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. Usingtrailing_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
The text was updated successfully, but these errors were encountered: