Skip to content

Commit 2cd0331

Browse files
committed
Move BitMask into Operators lint pass
1 parent 42492ec commit 2cd0331

File tree

8 files changed

+179
-166
lines changed

8 files changed

+179
-166
lines changed

clippy_lints/src/lib.register_all.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
1515
LintId::of(await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE),
1616
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
1717
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
18-
LintId::of(bit_mask::BAD_BIT_MASK),
19-
LintId::of(bit_mask::INEFFECTIVE_BIT_MASK),
2018
LintId::of(blacklisted_name::BLACKLISTED_NAME),
2119
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
2220
LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON),
@@ -252,6 +250,8 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
252250
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
253251
LintId::of(operators::ABSURD_EXTREME_COMPARISONS),
254252
LintId::of(operators::ASSIGN_OP_PATTERN),
253+
LintId::of(operators::BAD_BIT_MASK),
254+
LintId::of(operators::INEFFECTIVE_BIT_MASK),
255255
LintId::of(operators::MISREFACTORED_ASSIGN_OP),
256256
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
257257
LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),

clippy_lints/src/lib.register_correctness.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
88
LintId::of(attrs::DEPRECATED_SEMVER),
99
LintId::of(attrs::MISMATCHED_TARGET_OS),
1010
LintId::of(attrs::USELESS_ATTRIBUTE),
11-
LintId::of(bit_mask::BAD_BIT_MASK),
12-
LintId::of(bit_mask::INEFFECTIVE_BIT_MASK),
1311
LintId::of(booleans::LOGIC_BUG),
1412
LintId::of(casts::CAST_REF_TO_MUT),
1513
LintId::of(casts::CAST_SLICE_DIFFERENT_SIZES),
@@ -51,6 +49,8 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
5149
LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
5250
LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
5351
LintId::of(operators::ABSURD_EXTREME_COMPARISONS),
52+
LintId::of(operators::BAD_BIT_MASK),
53+
LintId::of(operators::INEFFECTIVE_BIT_MASK),
5454
LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
5555
LintId::of(ptr::INVALID_NULL_PTR_USAGE),
5656
LintId::of(ptr::MUT_FROM_REF),

clippy_lints/src/lib.register_lints.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,6 @@ store.register_lints(&[
5151
await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE,
5252
await_holding_invalid::AWAIT_HOLDING_LOCK,
5353
await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
54-
bit_mask::BAD_BIT_MASK,
55-
bit_mask::INEFFECTIVE_BIT_MASK,
56-
bit_mask::VERBOSE_BIT_MASK,
5754
blacklisted_name::BLACKLISTED_NAME,
5855
blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS,
5956
bool_assert_comparison::BOOL_ASSERT_COMPARISON,
@@ -422,9 +419,12 @@ store.register_lints(&[
422419
open_options::NONSENSICAL_OPEN_OPTIONS,
423420
operators::ABSURD_EXTREME_COMPARISONS,
424421
operators::ASSIGN_OP_PATTERN,
422+
operators::BAD_BIT_MASK,
425423
operators::FLOAT_ARITHMETIC,
424+
operators::INEFFECTIVE_BIT_MASK,
426425
operators::INTEGER_ARITHMETIC,
427426
operators::MISREFACTORED_ASSIGN_OP,
427+
operators::VERBOSE_BIT_MASK,
428428
option_env_unwrap::OPTION_ENV_UNWRAP,
429429
option_if_let_else::OPTION_IF_LET_ELSE,
430430
overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL,

clippy_lints/src/lib.register_pedantic.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
66
LintId::of(attrs::INLINE_ALWAYS),
7-
LintId::of(bit_mask::VERBOSE_BIT_MASK),
87
LintId::of(borrow_as_ptr::BORROW_AS_PTR),
98
LintId::of(bytecount::NAIVE_BYTECOUNT),
109
LintId::of(case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS),
@@ -75,6 +74,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
7574
LintId::of(no_effect::NO_EFFECT_UNDERSCORE_BINDING),
7675
LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
7776
LintId::of(non_expressive_names::SIMILAR_NAMES),
77+
LintId::of(operators::VERBOSE_BIT_MASK),
7878
LintId::of(pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE),
7979
LintId::of(pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF),
8080
LintId::of(ranges::RANGE_MINUS_ONE),

clippy_lints/src/lib.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ mod assertions_on_constants;
175175
mod async_yields_async;
176176
mod attrs;
177177
mod await_holding_invalid;
178-
mod bit_mask;
179178
mod blacklisted_name;
180179
mod blocks_in_if_conditions;
181180
mod bool_assert_comparison;
@@ -547,8 +546,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
547546
store.register_late_pass(|| Box::new(eq_op::EqOp));
548547
store.register_late_pass(|| Box::new(enum_clike::UnportableVariant));
549548
store.register_late_pass(|| Box::new(float_literal::FloatLiteral));
550-
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
551-
store.register_late_pass(move || Box::new(bit_mask::BitMask::new(verbose_bit_mask_threshold)));
552549
store.register_late_pass(|| Box::new(ptr::Ptr));
553550
store.register_late_pass(|| Box::new(ptr_eq::PtrEq));
554551
store.register_late_pass(|| Box::new(needless_bool::NeedlessBool));
@@ -908,7 +905,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
908905
store.register_late_pass(|| Box::new(get_first::GetFirst));
909906
store.register_early_pass(|| Box::new(unused_rounding::UnusedRounding));
910907
store.register_early_pass(move || Box::new(almost_complete_letter_range::AlmostCompleteLetterRange::new(msrv)));
911-
store.register_late_pass(|| Box::new(operators::Operators::default()));
908+
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
909+
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
912910
// add lints here, do not remove this comment, it's used in `new_lint`
913911
}
914912

clippy_lints/src/bit_mask.rs renamed to clippy_lints/src/operators/bit_mask.rs

Lines changed: 15 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -1,161 +1,23 @@
11
use clippy_utils::consts::{constant, Constant};
2-
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
3-
use clippy_utils::sugg::Sugg;
4-
use rustc_ast::ast::LitKind;
5-
use rustc_errors::Applicability;
2+
use clippy_utils::diagnostics::span_lint;
63
use rustc_hir::{BinOpKind, Expr, ExprKind};
7-
use rustc_lint::{LateContext, LateLintPass};
8-
use rustc_session::{declare_tool_lint, impl_lint_pass};
4+
use rustc_lint::LateContext;
95
use rustc_span::source_map::Span;
106

11-
declare_clippy_lint! {
12-
/// ### What it does
13-
/// Checks for incompatible bit masks in comparisons.
14-
///
15-
/// The formula for detecting if an expression of the type `_ <bit_op> m
16-
/// <cmp_op> c` (where `<bit_op>` is one of {`&`, `|`} and `<cmp_op>` is one of
17-
/// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following
18-
/// table:
19-
///
20-
/// |Comparison |Bit Op|Example |is always|Formula |
21-
/// |------------|------|-------------|---------|----------------------|
22-
/// |`==` or `!=`| `&` |`x & 2 == 3` |`false` |`c & m != c` |
23-
/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` |
24-
/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` |
25-
/// |`==` or `!=`| `\|` |`x \| 1 == 0`|`false` |`c \| m != c` |
26-
/// |`<` or `>=`| `\|` |`x \| 1 < 1` |`false` |`m >= c` |
27-
/// |`<=` or `>` | `\|` |`x \| 1 > 0` |`true` |`m > c` |
28-
///
29-
/// ### Why is this bad?
30-
/// If the bits that the comparison cares about are always
31-
/// set to zero or one by the bit mask, the comparison is constant `true` or
32-
/// `false` (depending on mask, compared value, and operators).
33-
///
34-
/// So the code is actively misleading, and the only reason someone would write
35-
/// this intentionally is to win an underhanded Rust contest or create a
36-
/// test-case for this lint.
37-
///
38-
/// ### Example
39-
/// ```rust
40-
/// # let x = 1;
41-
/// if (x & 1 == 2) { }
42-
/// ```
43-
#[clippy::version = "pre 1.29.0"]
44-
pub BAD_BIT_MASK,
45-
correctness,
46-
"expressions of the form `_ & mask == select` that will only ever return `true` or `false`"
47-
}
48-
49-
declare_clippy_lint! {
50-
/// ### What it does
51-
/// Checks for bit masks in comparisons which can be removed
52-
/// without changing the outcome. The basic structure can be seen in the
53-
/// following table:
54-
///
55-
/// |Comparison| Bit Op |Example |equals |
56-
/// |----------|----------|------------|-------|
57-
/// |`>` / `<=`|`\|` / `^`|`x \| 2 > 3`|`x > 3`|
58-
/// |`<` / `>=`|`\|` / `^`|`x ^ 1 < 4` |`x < 4`|
59-
///
60-
/// ### Why is this bad?
61-
/// Not equally evil as [`bad_bit_mask`](#bad_bit_mask),
62-
/// but still a bit misleading, because the bit mask is ineffective.
63-
///
64-
/// ### Known problems
65-
/// False negatives: This lint will only match instances
66-
/// where we have figured out the math (which is for a power-of-two compared
67-
/// value). This means things like `x | 1 >= 7` (which would be better written
68-
/// as `x >= 6`) will not be reported (but bit masks like this are fairly
69-
/// uncommon).
70-
///
71-
/// ### Example
72-
/// ```rust
73-
/// # let x = 1;
74-
/// if (x | 1 > 3) { }
75-
/// ```
76-
#[clippy::version = "pre 1.29.0"]
77-
pub INEFFECTIVE_BIT_MASK,
78-
correctness,
79-
"expressions where a bit mask will be rendered useless by a comparison, e.g., `(x | 1) > 2`"
80-
}
81-
82-
declare_clippy_lint! {
83-
/// ### What it does
84-
/// Checks for bit masks that can be replaced by a call
85-
/// to `trailing_zeros`
86-
///
87-
/// ### Why is this bad?
88-
/// `x.trailing_zeros() > 4` is much clearer than `x & 15
89-
/// == 0`
90-
///
91-
/// ### Known problems
92-
/// llvm generates better code for `x & 15 == 0` on x86
93-
///
94-
/// ### Example
95-
/// ```rust
96-
/// # let x = 1;
97-
/// if x & 0b1111 == 0 { }
98-
/// ```
99-
#[clippy::version = "pre 1.29.0"]
100-
pub VERBOSE_BIT_MASK,
101-
pedantic,
102-
"expressions where a bit mask is less readable than the corresponding method call"
103-
}
7+
use super::{BAD_BIT_MASK, INEFFECTIVE_BIT_MASK};
1048

105-
#[derive(Copy, Clone)]
106-
pub struct BitMask {
107-
verbose_bit_mask_threshold: u64,
108-
}
109-
110-
impl BitMask {
111-
#[must_use]
112-
pub fn new(verbose_bit_mask_threshold: u64) -> Self {
113-
Self {
114-
verbose_bit_mask_threshold,
115-
}
116-
}
117-
}
118-
119-
impl_lint_pass!(BitMask => [BAD_BIT_MASK, INEFFECTIVE_BIT_MASK, VERBOSE_BIT_MASK]);
120-
121-
impl<'tcx> LateLintPass<'tcx> for BitMask {
122-
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
123-
if let ExprKind::Binary(cmp, left, right) = &e.kind {
124-
if cmp.node.is_comparison() {
125-
if let Some(cmp_opt) = fetch_int_literal(cx, right) {
126-
check_compare(cx, left, cmp.node, cmp_opt, e.span);
127-
} else if let Some(cmp_val) = fetch_int_literal(cx, left) {
128-
check_compare(cx, right, invert_cmp(cmp.node), cmp_val, e.span);
129-
}
130-
}
131-
}
132-
133-
if let ExprKind::Binary(op, left, right) = &e.kind
134-
&& BinOpKind::Eq == op.node
135-
&& let ExprKind::Binary(op1, left1, right1) = &left.kind
136-
&& BinOpKind::BitAnd == op1.node
137-
&& let ExprKind::Lit(lit) = &right1.kind
138-
&& let LitKind::Int(n, _) = lit.node
139-
&& let ExprKind::Lit(lit1) = &right.kind
140-
&& let LitKind::Int(0, _) = lit1.node
141-
&& n.leading_zeros() == n.count_zeros()
142-
&& n > u128::from(self.verbose_bit_mask_threshold)
143-
{
144-
span_lint_and_then(
145-
cx,
146-
VERBOSE_BIT_MASK,
147-
e.span,
148-
"bit mask could be simplified with a call to `trailing_zeros`",
149-
|diag| {
150-
let sugg = Sugg::hir(cx, left1, "...").maybe_par();
151-
diag.span_suggestion(
152-
e.span,
153-
"try",
154-
format!("{}.trailing_zeros() >= {}", sugg, n.count_ones()),
155-
Applicability::MaybeIncorrect,
156-
);
157-
},
158-
);
9+
pub(super) fn check<'tcx>(
10+
cx: &LateContext<'tcx>,
11+
e: &'tcx Expr<'_>,
12+
op: BinOpKind,
13+
left: &'tcx Expr<'_>,
14+
right: &'tcx Expr<'_>,
15+
) {
16+
if op.is_comparison() {
17+
if let Some(cmp_opt) = fetch_int_literal(cx, right) {
18+
check_compare(cx, left, op, cmp_opt, e.span);
19+
} else if let Some(cmp_val) = fetch_int_literal(cx, left) {
20+
check_compare(cx, right, invert_cmp(op), cmp_val, e.span);
15921
}
16022
}
16123
}

0 commit comments

Comments
 (0)