X-Git-Url: https://git.lizzy.rs/?a=blobdiff_plain;f=clippy_lints%2Fsrc%2Fbit_mask.rs;h=f7daf3dab49481848e7d8e1fa3b0a2280bea109b;hb=dc4ea800b7126e0751ba75eae095cc2a805dc8da;hp=9af80493af1c366d69bb2933979f7140ee8c0d58;hpb=529f698c23099cbdb0afe5fe308814639808233d;p=rust.git diff --git a/clippy_lints/src/bit_mask.rs b/clippy_lints/src/bit_mask.rs index 9af80493af1..f7daf3dab49 100644 --- a/clippy_lints/src/bit_mask.rs +++ b/clippy_lints/src/bit_mask.rs @@ -1,102 +1,96 @@ -// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT -// file at the top-level directory of this distribution. -// -// Licensed under the Apache License, Version 2.0 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - use crate::consts::{constant, Constant}; -use crate::utils::sugg::Sugg; -use crate::utils::{span_lint, span_lint_and_then}; +use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; +use clippy_utils::sugg::Sugg; use if_chain::if_chain; -use rustc::hir::*; -use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; -use rustc::{declare_tool_lint, lint_array}; +use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use syntax::ast::LitKind; -use syntax::source_map::Span; +use rustc_hir::{BinOpKind, Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::source_map::Span; -/// **What it does:** Checks for incompatible bit masks in comparisons. -/// -/// The formula for detecting if an expression of the type `_ m -/// c` (where `` is one of {`&`, `|`} and `` is one of -/// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following -/// table: -/// -/// |Comparison |Bit Op|Example |is always|Formula | -/// |------------|------|------------|---------|----------------------| -/// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | -/// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | -/// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | -/// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | -/// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | -/// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | -/// -/// **Why is this bad?** If the bits that the comparison cares about are always -/// set to zero or one by the bit mask, the comparison is constant `true` or -/// `false` (depending on mask, compared value, and operators). -/// -/// So the code is actively misleading, and the only reason someone would write -/// this intentionally is to win an underhanded Rust contest or create a -/// test-case for this lint. -/// -/// **Known problems:** None. -/// -/// **Example:** -/// ```rust -/// if (x & 1 == 2) { … } -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for incompatible bit masks in comparisons. + /// + /// The formula for detecting if an expression of the type `_ m + /// c` (where `` is one of {`&`, `|`} and `` is one of + /// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following + /// table: + /// + /// |Comparison |Bit Op|Example |is always|Formula | + /// |------------|------|------------|---------|----------------------| + /// |`==` or `!=`| `&` |`x & 2 == 3`|`false` |`c & m != c` | + /// |`<` or `>=`| `&` |`x & 2 < 3` |`true` |`m < c` | + /// |`>` or `<=`| `&` |`x & 1 > 1` |`false` |`m <= c` | + /// |`==` or `!=`| `|` |`x | 1 == 0`|`false` |`c | m != c` | + /// |`<` or `>=`| `|` |`x | 1 < 1` |`false` |`m >= c` | + /// |`<=` or `>` | `|` |`x | 1 > 0` |`true` |`m > c` | + /// + /// **Why is this bad?** If the bits that the comparison cares about are always + /// set to zero or one by the bit mask, the comparison is constant `true` or + /// `false` (depending on mask, compared value, and operators). + /// + /// So the code is actively misleading, and the only reason someone would write + /// this intentionally is to win an underhanded Rust contest or create a + /// test-case for this lint. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust + /// # let x = 1; + /// if (x & 1 == 2) { } + /// ``` pub BAD_BIT_MASK, correctness, "expressions of the form `_ & mask == select` that will only ever return `true` or `false`" } -/// **What it does:** Checks for bit masks in comparisons which can be removed -/// without changing the outcome. The basic structure can be seen in the -/// following table: -/// -/// |Comparison| Bit Op |Example |equals | -/// |----------|---------|-----------|-------| -/// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`| -/// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`| -/// -/// **Why is this bad?** Not equally evil as [`bad_bit_mask`](#bad_bit_mask), -/// but still a bit misleading, because the bit mask is ineffective. -/// -/// **Known problems:** False negatives: This lint will only match instances -/// where we have figured out the math (which is for a power-of-two compared -/// value). This means things like `x | 1 >= 7` (which would be better written -/// as `x >= 6`) will not be reported (but bit masks like this are fairly -/// uncommon). -/// -/// **Example:** -/// ```rust -/// if (x | 1 > 3) { … } -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for bit masks in comparisons which can be removed + /// without changing the outcome. The basic structure can be seen in the + /// following table: + /// + /// |Comparison| Bit Op |Example |equals | + /// |----------|---------|-----------|-------| + /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`| + /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`| + /// + /// **Why is this bad?** Not equally evil as [`bad_bit_mask`](#bad_bit_mask), + /// but still a bit misleading, because the bit mask is ineffective. + /// + /// **Known problems:** False negatives: This lint will only match instances + /// where we have figured out the math (which is for a power-of-two compared + /// value). This means things like `x | 1 >= 7` (which would be better written + /// as `x >= 6`) will not be reported (but bit masks like this are fairly + /// uncommon). + /// + /// **Example:** + /// ```rust + /// # let x = 1; + /// if (x | 1 > 3) { } + /// ``` pub INEFFECTIVE_BIT_MASK, correctness, - "expressions where a bit mask will be rendered useless by a comparison, e.g. `(x | 1) > 2`" + "expressions where a bit mask will be rendered useless by a comparison, e.g., `(x | 1) > 2`" } -/// **What it does:** Checks for bit masks that can be replaced by a call -/// to `trailing_zeros` -/// -/// **Why is this bad?** `x.trailing_zeros() > 4` is much clearer than `x & 15 -/// == 0` -/// -/// **Known problems:** llvm generates better code for `x & 15 == 0` on x86 -/// -/// **Example:** -/// ```rust -/// x & 0x1111 == 0 -/// ``` declare_clippy_lint! { + /// **What it does:** Checks for bit masks that can be replaced by a call + /// to `trailing_zeros` + /// + /// **Why is this bad?** `x.trailing_zeros() > 4` is much clearer than `x & 15 + /// == 0` + /// + /// **Known problems:** llvm generates better code for `x & 15 == 0` on x86 + /// + /// **Example:** + /// ```rust + /// # let x = 1; + /// if x & 0b1111 == 0 { } + /// ``` pub VERBOSE_BIT_MASK, - style, + pedantic, "expressions where a bit mask is less readable than the corresponding method call" } @@ -106,6 +100,7 @@ pub struct BitMask { } impl BitMask { + #[must_use] pub fn new(verbose_bit_mask_threshold: u64) -> Self { Self { verbose_bit_mask_threshold, @@ -113,15 +108,11 @@ pub fn new(verbose_bit_mask_threshold: u64) -> Self { } } -impl LintPass for BitMask { - fn get_lints(&self) -> LintArray { - lint_array!(BAD_BIT_MASK, INEFFECTIVE_BIT_MASK, VERBOSE_BIT_MASK) - } -} +impl_lint_pass!(BitMask => [BAD_BIT_MASK, INEFFECTIVE_BIT_MASK, VERBOSE_BIT_MASK]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BitMask { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { - if let ExprKind::Binary(cmp, left, right) = &e.node { +impl<'tcx> LateLintPass<'tcx> for BitMask { + fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { + if let ExprKind::Binary(cmp, left, right) = &e.kind { if cmp.node.is_comparison() { if let Some(cmp_opt) = fetch_int_literal(cx, right) { check_compare(cx, left, cmp.node, cmp_opt, e.span) @@ -131,13 +122,13 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { } } if_chain! { - if let ExprKind::Binary(op, left, right) = &e.node; + if let ExprKind::Binary(op, left, right) = &e.kind; if BinOpKind::Eq == op.node; - if let ExprKind::Binary(op1, left1, right1) = &left.node; + if let ExprKind::Binary(op1, left1, right1) = &left.kind; if BinOpKind::BitAnd == op1.node; - if let ExprKind::Lit(lit) = &right1.node; + if let ExprKind::Lit(lit) = &right1.kind; if let LitKind::Int(n, _) = lit.node; - if let ExprKind::Lit(lit1) = &right.node; + if let ExprKind::Lit(lit1) = &right.kind; if let LitKind::Int(0, _) = lit1.node; if n.leading_zeros() == n.count_zeros(); if n > u128::from(self.verbose_bit_mask_threshold); @@ -146,9 +137,9 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { VERBOSE_BIT_MASK, e.span, "bit mask could be simplified with a call to `trailing_zeros`", - |db| { + |diag| { let sugg = Sugg::hir(cx, left1, "...").maybe_par(); - db.span_suggestion_with_applicability( + diag.span_suggestion( e.span, "try", format!("{}.trailing_zeros() >= {}", sugg, n.count_ones()), @@ -160,6 +151,7 @@ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { } } +#[must_use] fn invert_cmp(cmp: BinOpKind) -> BinOpKind { match cmp { BinOpKind::Eq => BinOpKind::Eq, @@ -172,8 +164,8 @@ fn invert_cmp(cmp: BinOpKind) -> BinOpKind { } } -fn check_compare(cx: &LateContext<'_, '_>, bit_op: &Expr, cmp_op: BinOpKind, cmp_value: u128, span: Span) { - if let ExprKind::Binary(op, left, right) = &bit_op.node { +fn check_compare(cx: &LateContext<'_>, bit_op: &Expr<'_>, cmp_op: BinOpKind, cmp_value: u128, span: Span) { + if let ExprKind::Binary(op, left, right) = &bit_op.kind { if op.node != BinOpKind::BitAnd && op.node != BinOpKind::BitOr { return; } @@ -183,8 +175,9 @@ fn check_compare(cx: &LateContext<'_, '_>, bit_op: &Expr, cmp_op: BinOpKind, cmp } } +#[allow(clippy::too_many_lines)] fn check_bit_mask( - cx: &LateContext<'_, '_>, + cx: &LateContext<'_>, bit_op: BinOpKind, cmp_op: BinOpKind, mask_value: u128, @@ -297,7 +290,7 @@ fn check_bit_mask( } } -fn check_ineffective_lt(cx: &LateContext<'_, '_>, span: Span, m: u128, c: u128, op: &str) { +fn check_ineffective_lt(cx: &LateContext<'_>, span: Span, m: u128, c: u128, op: &str) { if c.is_power_of_two() && m < c { span_lint( cx, @@ -311,7 +304,7 @@ fn check_ineffective_lt(cx: &LateContext<'_, '_>, span: Span, m: u128, c: u128, } } -fn check_ineffective_gt(cx: &LateContext<'_, '_>, span: Span, m: u128, c: u128, op: &str) { +fn check_ineffective_gt(cx: &LateContext<'_>, span: Span, m: u128, c: u128, op: &str) { if (c + 1).is_power_of_two() && m <= c { span_lint( cx, @@ -325,8 +318,8 @@ fn check_ineffective_gt(cx: &LateContext<'_, '_>, span: Span, m: u128, c: u128, } } -fn fetch_int_literal(cx: &LateContext<'_, '_>, lit: &Expr) -> Option { - match constant(cx, cx.tables, lit)?.0 { +fn fetch_int_literal(cx: &LateContext<'_>, lit: &Expr<'_>) -> Option { + match constant(cx, cx.typeck_results(), lit)?.0 { Constant::Int(n) => Some(n), _ => None, }