]>
Commit | Line | Data |
---|---|---|
f20569fa XL |
1 | use crate::consts::{constant, Constant}; |
2 | use crate::utils::sugg::Sugg; | |
3 | use crate::utils::{span_lint, span_lint_and_then}; | |
4 | use if_chain::if_chain; | |
5 | use rustc_ast::ast::LitKind; | |
6 | use rustc_errors::Applicability; | |
7 | use rustc_hir::{BinOpKind, Expr, ExprKind}; | |
8 | use rustc_lint::{LateContext, LateLintPass}; | |
9 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
10 | use rustc_span::source_map::Span; | |
11 | ||
12 | declare_clippy_lint! { | |
13 | /// **What it does:** 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?** If the bits that the comparison cares about are always | |
30 | /// set to zero or one by the bit mask, the comparison is constant `true` or | |
31 | /// `false` (depending on mask, compared value, and operators). | |
32 | /// | |
33 | /// So the code is actively misleading, and the only reason someone would write | |
34 | /// this intentionally is to win an underhanded Rust contest or create a | |
35 | /// test-case for this lint. | |
36 | /// | |
37 | /// **Known problems:** None. | |
38 | /// | |
39 | /// **Example:** | |
40 | /// ```rust | |
41 | /// # let x = 1; | |
42 | /// if (x & 1 == 2) { } | |
43 | /// ``` | |
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:** Checks for bit masks in comparisons which can be removed | |
51 | /// without changing the outcome. The basic structure can be seen in the | |
52 | /// following table: | |
53 | /// | |
54 | /// |Comparison| Bit Op |Example |equals | | |
55 | /// |----------|---------|-----------|-------| | |
56 | /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`| | |
57 | /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`| | |
58 | /// | |
59 | /// **Why is this bad?** Not equally evil as [`bad_bit_mask`](#bad_bit_mask), | |
60 | /// but still a bit misleading, because the bit mask is ineffective. | |
61 | /// | |
62 | /// **Known problems:** False negatives: This lint will only match instances | |
63 | /// where we have figured out the math (which is for a power-of-two compared | |
64 | /// value). This means things like `x | 1 >= 7` (which would be better written | |
65 | /// as `x >= 6`) will not be reported (but bit masks like this are fairly | |
66 | /// uncommon). | |
67 | /// | |
68 | /// **Example:** | |
69 | /// ```rust | |
70 | /// # let x = 1; | |
71 | /// if (x | 1 > 3) { } | |
72 | /// ``` | |
73 | pub INEFFECTIVE_BIT_MASK, | |
74 | correctness, | |
75 | "expressions where a bit mask will be rendered useless by a comparison, e.g., `(x | 1) > 2`" | |
76 | } | |
77 | ||
78 | declare_clippy_lint! { | |
79 | /// **What it does:** Checks for bit masks that can be replaced by a call | |
80 | /// to `trailing_zeros` | |
81 | /// | |
82 | /// **Why is this bad?** `x.trailing_zeros() > 4` is much clearer than `x & 15 | |
83 | /// == 0` | |
84 | /// | |
85 | /// **Known problems:** llvm generates better code for `x & 15 == 0` on x86 | |
86 | /// | |
87 | /// **Example:** | |
88 | /// ```rust | |
89 | /// # let x = 1; | |
90 | /// if x & 0b1111 == 0 { } | |
91 | /// ``` | |
92 | pub VERBOSE_BIT_MASK, | |
93 | pedantic, | |
94 | "expressions where a bit mask is less readable than the corresponding method call" | |
95 | } | |
96 | ||
97 | #[derive(Copy, Clone)] | |
98 | pub struct BitMask { | |
99 | verbose_bit_mask_threshold: u64, | |
100 | } | |
101 | ||
102 | impl BitMask { | |
103 | #[must_use] | |
104 | pub fn new(verbose_bit_mask_threshold: u64) -> Self { | |
105 | Self { | |
106 | verbose_bit_mask_threshold, | |
107 | } | |
108 | } | |
109 | } | |
110 | ||
111 | impl_lint_pass!(BitMask => [BAD_BIT_MASK, INEFFECTIVE_BIT_MASK, VERBOSE_BIT_MASK]); | |
112 | ||
113 | impl<'tcx> LateLintPass<'tcx> for BitMask { | |
114 | fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { | |
115 | if let ExprKind::Binary(cmp, left, right) = &e.kind { | |
116 | if cmp.node.is_comparison() { | |
117 | if let Some(cmp_opt) = fetch_int_literal(cx, right) { | |
118 | check_compare(cx, left, cmp.node, cmp_opt, e.span) | |
119 | } else if let Some(cmp_val) = fetch_int_literal(cx, left) { | |
120 | check_compare(cx, right, invert_cmp(cmp.node), cmp_val, e.span) | |
121 | } | |
122 | } | |
123 | } | |
124 | if_chain! { | |
125 | if let ExprKind::Binary(op, left, right) = &e.kind; | |
126 | if BinOpKind::Eq == op.node; | |
127 | if let ExprKind::Binary(op1, left1, right1) = &left.kind; | |
128 | if BinOpKind::BitAnd == op1.node; | |
129 | if let ExprKind::Lit(lit) = &right1.kind; | |
130 | if let LitKind::Int(n, _) = lit.node; | |
131 | if let ExprKind::Lit(lit1) = &right.kind; | |
132 | if let LitKind::Int(0, _) = lit1.node; | |
133 | if n.leading_zeros() == n.count_zeros(); | |
134 | if n > u128::from(self.verbose_bit_mask_threshold); | |
135 | then { | |
136 | span_lint_and_then(cx, | |
137 | VERBOSE_BIT_MASK, | |
138 | e.span, | |
139 | "bit mask could be simplified with a call to `trailing_zeros`", | |
140 | |diag| { | |
141 | let sugg = Sugg::hir(cx, left1, "...").maybe_par(); | |
142 | diag.span_suggestion( | |
143 | e.span, | |
144 | "try", | |
145 | format!("{}.trailing_zeros() >= {}", sugg, n.count_ones()), | |
146 | Applicability::MaybeIncorrect, | |
147 | ); | |
148 | }); | |
149 | } | |
150 | } | |
151 | } | |
152 | } | |
153 | ||
154 | #[must_use] | |
155 | fn invert_cmp(cmp: BinOpKind) -> BinOpKind { | |
156 | match cmp { | |
157 | BinOpKind::Eq => BinOpKind::Eq, | |
158 | BinOpKind::Ne => BinOpKind::Ne, | |
159 | BinOpKind::Lt => BinOpKind::Gt, | |
160 | BinOpKind::Gt => BinOpKind::Lt, | |
161 | BinOpKind::Le => BinOpKind::Ge, | |
162 | BinOpKind::Ge => BinOpKind::Le, | |
163 | _ => BinOpKind::Or, // Dummy | |
164 | } | |
165 | } | |
166 | ||
167 | fn check_compare(cx: &LateContext<'_>, bit_op: &Expr<'_>, cmp_op: BinOpKind, cmp_value: u128, span: Span) { | |
168 | if let ExprKind::Binary(op, left, right) = &bit_op.kind { | |
169 | if op.node != BinOpKind::BitAnd && op.node != BinOpKind::BitOr { | |
170 | return; | |
171 | } | |
172 | fetch_int_literal(cx, right) | |
173 | .or_else(|| fetch_int_literal(cx, left)) | |
174 | .map_or((), |mask| check_bit_mask(cx, op.node, cmp_op, mask, cmp_value, span)) | |
175 | } | |
176 | } | |
177 | ||
178 | #[allow(clippy::too_many_lines)] | |
179 | fn check_bit_mask( | |
180 | cx: &LateContext<'_>, | |
181 | bit_op: BinOpKind, | |
182 | cmp_op: BinOpKind, | |
183 | mask_value: u128, | |
184 | cmp_value: u128, | |
185 | span: Span, | |
186 | ) { | |
187 | match cmp_op { | |
188 | BinOpKind::Eq | BinOpKind::Ne => match bit_op { | |
189 | BinOpKind::BitAnd => { | |
190 | if mask_value & cmp_value != cmp_value { | |
191 | if cmp_value != 0 { | |
192 | span_lint( | |
193 | cx, | |
194 | BAD_BIT_MASK, | |
195 | span, | |
196 | &format!( | |
197 | "incompatible bit mask: `_ & {}` can never be equal to `{}`", | |
198 | mask_value, cmp_value | |
199 | ), | |
200 | ); | |
201 | } | |
202 | } else if mask_value == 0 { | |
203 | span_lint(cx, BAD_BIT_MASK, span, "&-masking with zero"); | |
204 | } | |
205 | }, | |
206 | BinOpKind::BitOr => { | |
207 | if mask_value | cmp_value != cmp_value { | |
208 | span_lint( | |
209 | cx, | |
210 | BAD_BIT_MASK, | |
211 | span, | |
212 | &format!( | |
213 | "incompatible bit mask: `_ | {}` can never be equal to `{}`", | |
214 | mask_value, cmp_value | |
215 | ), | |
216 | ); | |
217 | } | |
218 | }, | |
219 | _ => (), | |
220 | }, | |
221 | BinOpKind::Lt | BinOpKind::Ge => match bit_op { | |
222 | BinOpKind::BitAnd => { | |
223 | if mask_value < cmp_value { | |
224 | span_lint( | |
225 | cx, | |
226 | BAD_BIT_MASK, | |
227 | span, | |
228 | &format!( | |
229 | "incompatible bit mask: `_ & {}` will always be lower than `{}`", | |
230 | mask_value, cmp_value | |
231 | ), | |
232 | ); | |
233 | } else if mask_value == 0 { | |
234 | span_lint(cx, BAD_BIT_MASK, span, "&-masking with zero"); | |
235 | } | |
236 | }, | |
237 | BinOpKind::BitOr => { | |
238 | if mask_value >= cmp_value { | |
239 | span_lint( | |
240 | cx, | |
241 | BAD_BIT_MASK, | |
242 | span, | |
243 | &format!( | |
244 | "incompatible bit mask: `_ | {}` will never be lower than `{}`", | |
245 | mask_value, cmp_value | |
246 | ), | |
247 | ); | |
248 | } else { | |
249 | check_ineffective_lt(cx, span, mask_value, cmp_value, "|"); | |
250 | } | |
251 | }, | |
252 | BinOpKind::BitXor => check_ineffective_lt(cx, span, mask_value, cmp_value, "^"), | |
253 | _ => (), | |
254 | }, | |
255 | BinOpKind::Le | BinOpKind::Gt => match bit_op { | |
256 | BinOpKind::BitAnd => { | |
257 | if mask_value <= cmp_value { | |
258 | span_lint( | |
259 | cx, | |
260 | BAD_BIT_MASK, | |
261 | span, | |
262 | &format!( | |
263 | "incompatible bit mask: `_ & {}` will never be higher than `{}`", | |
264 | mask_value, cmp_value | |
265 | ), | |
266 | ); | |
267 | } else if mask_value == 0 { | |
268 | span_lint(cx, BAD_BIT_MASK, span, "&-masking with zero"); | |
269 | } | |
270 | }, | |
271 | BinOpKind::BitOr => { | |
272 | if mask_value > cmp_value { | |
273 | span_lint( | |
274 | cx, | |
275 | BAD_BIT_MASK, | |
276 | span, | |
277 | &format!( | |
278 | "incompatible bit mask: `_ | {}` will always be higher than `{}`", | |
279 | mask_value, cmp_value | |
280 | ), | |
281 | ); | |
282 | } else { | |
283 | check_ineffective_gt(cx, span, mask_value, cmp_value, "|"); | |
284 | } | |
285 | }, | |
286 | BinOpKind::BitXor => check_ineffective_gt(cx, span, mask_value, cmp_value, "^"), | |
287 | _ => (), | |
288 | }, | |
289 | _ => (), | |
290 | } | |
291 | } | |
292 | ||
293 | fn check_ineffective_lt(cx: &LateContext<'_>, span: Span, m: u128, c: u128, op: &str) { | |
294 | if c.is_power_of_two() && m < c { | |
295 | span_lint( | |
296 | cx, | |
297 | INEFFECTIVE_BIT_MASK, | |
298 | span, | |
299 | &format!( | |
300 | "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", | |
301 | op, m, c | |
302 | ), | |
303 | ); | |
304 | } | |
305 | } | |
306 | ||
307 | fn check_ineffective_gt(cx: &LateContext<'_>, span: Span, m: u128, c: u128, op: &str) { | |
308 | if (c + 1).is_power_of_two() && m <= c { | |
309 | span_lint( | |
310 | cx, | |
311 | INEFFECTIVE_BIT_MASK, | |
312 | span, | |
313 | &format!( | |
314 | "ineffective bit mask: `x {} {}` compared to `{}`, is the same as x compared directly", | |
315 | op, m, c | |
316 | ), | |
317 | ); | |
318 | } | |
319 | } | |
320 | ||
321 | fn fetch_int_literal(cx: &LateContext<'_>, lit: &Expr<'_>) -> Option<u128> { | |
322 | match constant(cx, cx.typeck_results(), lit)?.0 { | |
323 | Constant::Int(n) => Some(n), | |
324 | _ => None, | |
325 | } | |
326 | } |