Skip to content

Commit 6da9f49

Browse files
authored
Lint bit width (#16902)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16902)* Fixes #16876 Current state: - [x] suggests change of `Uint::BITS - x.leading_zeros()` to `x.bit_width()` - [x] suggests change of `Int::BITS - x.leading_zeros()` to `x.cast_unsigned().bit_width()` - [x] suggests change of `NonZero::<Uint>::BITS - x.leading_zeros()` to `x.bit_width().get()` - [x] suggests change of `NonZero::<Int>::BITS - x.leading_zeros()` to `x.cast_unsigned().bit_width().get()` - [x] when the calling Type of `T::BITS` does not align with value (`x`) calling `leading_zeros()` the lint will raise this mismatch and suggest to replace the whole line with `x.bit_width()` ## Description rust 1.97 introduces the method `bit_width()` for uints and NonZero ([docs](https://doc.rust-lang.org/nightly/core/primitive.u32.html?search=bit_width)), this will make the manual computation redundant and unnecessary. ## Example 1 ```rust let x: u32 = b'101'; let bit_width = u32::BITS - x.leading_zeros(); ``` Can be replaced with: ```rust let x: u32 = b'101'; let bit_width = x.bit_width(); ``` ## Example 2 ```rust let y = NonZero::<u32>::new(5).unwrap(); let _ = NonZero::<u32>::BITS - y.leading_zeros(); ``` Can be replaced with: ```rust let y = NonZero::<u32>::new(5).unwrap(); let bit_width = y.bit_width().get(); ``` ## Example 3 ```rust let y: i32 = 5; let _ = i32::BITS - y.leading_zeros(); ``` Can be replaced with: ```rust let y: i32 = 5; let _ = y.cast_unsigned().bit_width(); ``` ## Example 4 ```rust let y = NonZero::<i32>::new(5).unwrap(); let _ = NonZero::<i32>::BITS - y.leading_zeros(); ``` Can be replaced with: ```rust let y = NonZero::<i32>::new(5).unwrap(); let bit_width = y.cast_unsigned().bit_width().get(); ``` ## Example 5 ```rust let y: u32 = 5; let _ = NonZero::<u64>::BITS - y.leading_zeros(); ``` Can be replaced with: ```rust let y: u32 = 5; let bit_width = y.bit_width(); ``` changelog: [`manual_bit_width`], [`mismatched_bit_width_type`]: Added a lint to detect bit_width implementations. I have - \[x] Followed [lint naming conventions][lint_naming] - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - \[x] Added lint documentation - \[x] Run `cargo dev fmt`
2 parents 60f8c3a + a35fed2 commit 6da9f49

12 files changed

Lines changed: 1243 additions & 1 deletion

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6979,6 +6979,7 @@ Released 2018-09-13
69796979
[`manual_assert`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert
69806980
[`manual_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq
69816981
[`manual_async_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_async_fn
6982+
[`manual_bit_width`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bit_width
69826983
[`manual_bits`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_bits
69836984
[`manual_c_str_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_c_str_literals
69846985
[`manual_checked_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_checked_ops
@@ -7071,6 +7072,7 @@ Released 2018-09-13
70717072
[`min_ident_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_ident_chars
70727073
[`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
70737074
[`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
7075+
[`mismatched_bit_width_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_bit_width_type
70747076
[`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os
70757077
[`mismatching_type_param_order`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatching_type_param_order
70767078
[`misnamed_getters`]: https://rust-lang.github.io/rust-clippy/master/index.html#misnamed_getters

clippy_lints/src/bit_width.rs

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
use clippy_config::Conf;
2+
use clippy_utils::diagnostics::span_lint_and_then;
3+
use clippy_utils::msrvs::{self, Msrv};
4+
use clippy_utils::source::snippet_with_context;
5+
use clippy_utils::{is_from_proc_macro, sym};
6+
use rustc_errors::Applicability;
7+
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
8+
use rustc_lint::{LateContext, LateLintPass, LintContext};
9+
use rustc_middle::ty::{self, Ty};
10+
use rustc_session::impl_lint_pass;
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// Checks for usage of `T::BITS - x.leading_zeros()` when `x.bit_width()` is available.
15+
///
16+
/// ### Why is this bad?
17+
/// Manual reimplementations of `bit_width` increase code complexity for little benefit.
18+
///
19+
/// ### Example
20+
/// ```no_run
21+
/// let x: u32 = 5;
22+
/// let bit_width = u32::BITS - x.leading_zeros();
23+
/// ```
24+
/// Use instead:
25+
/// ```no_run
26+
/// let x: u32 = 5;
27+
/// let bit_width = x.bit_width();
28+
/// ```
29+
#[clippy::version = "1.98.0"]
30+
pub MANUAL_BIT_WIDTH,
31+
pedantic,
32+
"manually reimplementing `bit_width`"
33+
}
34+
35+
declare_clippy_lint! {
36+
/// ### What it does
37+
/// Checks for usage of `T::BITS - x.leading_zeros()` where T and x are of different types.
38+
///
39+
/// ### Why is this bad?
40+
/// Substracting `leading_zeros` from the number of bits of another type might be
41+
/// a buggy implementation of the `bit_width` method.
42+
///
43+
/// ### Example
44+
/// ```no_run
45+
/// let x: u64 = 5;
46+
/// let bit_width = u32::BITS - x.leading_zeros();
47+
/// ```
48+
/// Use instead:
49+
/// ```no_run
50+
/// let x: u64 = 5;
51+
/// let bit_width = x.bit_width();
52+
/// ```
53+
#[clippy::version = "1.98.0"]
54+
pub MISMATCHED_BIT_WIDTH_TYPE,
55+
suspicious,
56+
"type mismatch in bit width calculation"
57+
}
58+
59+
impl_lint_pass!(ManualBitWidth => [MANUAL_BIT_WIDTH, MISMATCHED_BIT_WIDTH_TYPE]);
60+
61+
#[derive(Clone, Copy, PartialEq)]
62+
enum IntKind<'a> {
63+
Int(ty::IntTy),
64+
Uint(ty::UintTy),
65+
// NOTE: in the following two variants, the inner `Ty` stores the entire `NonZero<T>`
66+
// and not just `T`. This is so that we can print it in the suggestion.
67+
NonZero(Ty<'a>),
68+
NonZeroU(Ty<'a>),
69+
}
70+
71+
impl IntKind<'_> {
72+
fn inner_ty(self) -> String {
73+
match self {
74+
Self::Int(ty) => ty.name_str().to_string(),
75+
Self::Uint(ty) => ty.name_str().to_string(),
76+
Self::NonZero(ty) | Self::NonZeroU(ty) => ty.to_string(),
77+
}
78+
}
79+
80+
fn suggestion(&self) -> &'static str {
81+
match self {
82+
Self::Int(_) => ".cast_unsigned().bit_width()",
83+
Self::Uint(_) => ".bit_width()",
84+
Self::NonZero(_) => ".cast_unsigned().bit_width().get()",
85+
Self::NonZeroU(_) => ".bit_width().get()",
86+
}
87+
}
88+
}
89+
90+
pub struct ManualBitWidth {
91+
msrv: Msrv,
92+
}
93+
94+
impl ManualBitWidth {
95+
pub fn new(conf: &Conf) -> Self {
96+
Self { msrv: conf.msrv }
97+
}
98+
}
99+
100+
impl LateLintPass<'_> for ManualBitWidth {
101+
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
102+
if expr.span.in_external_macro(cx.sess().source_map()) {
103+
return;
104+
}
105+
106+
match expr.kind {
107+
// `T::BITS - n.leading_zeros()`
108+
ExprKind::Binary(op, left, right)
109+
if op.node == BinOpKind::Sub
110+
&& let ExprKind::MethodCall(leading_zeros, recv, [], _) = right.kind
111+
&& leading_zeros.ident.name == sym::leading_zeros
112+
&& let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = left.kind
113+
&& segment.ident.name == sym::BITS
114+
&& let right_ty = cx.typeck_results().expr_ty(recv)
115+
&& let Some(right_int_kind) = get_int_kind(cx, right_ty)
116+
&& let left_ty = cx.typeck_results().node_type(hir_ty.hir_id)
117+
&& let Some(left_int_kind) = get_int_kind(cx, left_ty)
118+
&& self.msrv.meets(cx, msrvs::BIT_WIDTH)
119+
&& left.span.eq_ctxt(right.span)
120+
&& !is_from_proc_macro(cx, expr) =>
121+
{
122+
if left_int_kind == right_int_kind {
123+
// manual implementation of bit_width
124+
emit_manual_bit_width(cx, recv, expr, right_int_kind);
125+
} else {
126+
// mismatched calling types
127+
emit_type_mismatch(cx, recv, expr, right_int_kind);
128+
}
129+
},
130+
_ => {},
131+
}
132+
}
133+
}
134+
135+
fn get_int_kind<'a>(cx: &LateContext<'a>, ty: Ty<'a>) -> Option<IntKind<'a>> {
136+
match ty.kind() {
137+
// int::BITS or uint::BITS
138+
ty::Int(int_ty) => Some(IntKind::Int(*int_ty)),
139+
ty::Uint(uint_ty) => Some(IntKind::Uint(*uint_ty)),
140+
// NonZero::<int/uint>::BITS
141+
ty::Adt(adt, args) if cx.tcx.is_diagnostic_item(sym::NonZero, adt.did()) => {
142+
let arg = args.type_at(0);
143+
match arg.kind() {
144+
ty::Int(_) => Some(IntKind::NonZero(ty)),
145+
ty::Uint(_) => Some(IntKind::NonZeroU(ty)),
146+
_ => None,
147+
}
148+
},
149+
_ => None,
150+
}
151+
}
152+
153+
fn emit_manual_bit_width(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>, ty_kind: IntKind<'_>) {
154+
span_lint_and_then(
155+
cx,
156+
MANUAL_BIT_WIDTH,
157+
full_expr.span,
158+
"manual implementation of `bit_width`",
159+
|diag| {
160+
let mut app = Applicability::MachineApplicable;
161+
let (recv_snip, _) = snippet_with_context(cx, recv.span, full_expr.span.ctxt(), "_", &mut app);
162+
let suggestion = ty_kind.suggestion();
163+
164+
diag.span_suggestion_verbose(full_expr.span, "try", format!("{recv_snip}{suggestion}"), app);
165+
},
166+
);
167+
}
168+
169+
fn emit_type_mismatch(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>, ty_kind: IntKind<'_>) {
170+
span_lint_and_then(
171+
cx,
172+
MISMATCHED_BIT_WIDTH_TYPE,
173+
full_expr.span,
174+
"possible buggy implementation of `bit_width`",
175+
|diag| {
176+
diag.note("in order to calculate the bit width, `T::BITS` should match the type of the value calling `.leading_zeros()`");
177+
178+
let mut app = Applicability::MaybeIncorrect;
179+
let (recv_snip, _) = snippet_with_context(cx, recv.span, full_expr.span.ctxt(), "_", &mut app);
180+
let suggestion = ty_kind.suggestion();
181+
let x_ty = ty_kind.inner_ty();
182+
183+
diag.span_suggestion_verbose(
184+
full_expr.span,
185+
format!("if you meant to use `{x_ty}::BITS`, use"),
186+
format!("{recv_snip}{suggestion}"),
187+
app,
188+
);
189+
},
190+
);
191+
}

clippy_lints/src/declared_lints.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
3333
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
3434
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,
3535
crate::await_holding_invalid::AWAIT_HOLDING_REFCELL_REF_INFO,
36+
crate::bit_width::MANUAL_BIT_WIDTH_INFO,
37+
crate::bit_width::MISMATCHED_BIT_WIDTH_TYPE_INFO,
3638
crate::blocks_in_conditions::BLOCKS_IN_CONDITIONS_INFO,
3739
crate::bool_assert_comparison::BOOL_ASSERT_COMPARISON_INFO,
3840
crate::bool_comparison::BOOL_COMPARISON_INFO,

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ mod assigning_clones;
7575
mod async_yields_async;
7676
mod attrs;
7777
mod await_holding_invalid;
78+
mod bit_width;
7879
mod blocks_in_conditions;
7980
mod bool_assert_comparison;
8081
mod bool_comparison;
@@ -733,6 +734,7 @@ rustc_lint::late_lint_methods!(
733734
NeedlessLateInit: needless_late_init::NeedlessLateInit<'tcx> = needless_late_init::NeedlessLateInit::new(conf),
734735
ReturnSelfNotMustUse: return_self_not_must_use::ReturnSelfNotMustUse = return_self_not_must_use::ReturnSelfNotMustUse,
735736
NumberedFields: init_numbered_fields::NumberedFields = init_numbered_fields::NumberedFields,
737+
ManualBitWidth: bit_width::ManualBitWidth = bit_width::ManualBitWidth::new(conf),
736738
ManualBits: manual_bits::ManualBits = manual_bits::ManualBits::new(conf),
737739
DefaultUnionRepresentation: default_union_representation::DefaultUnionRepresentation = default_union_representation::DefaultUnionRepresentation,
738740
OnlyUsedInRecursion: only_used_in_recursion::OnlyUsedInRecursion = <only_used_in_recursion::OnlyUsedInRecursion>::default(),

clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ macro_rules! msrv_aliases {
2424

2525
// names may refer to stabilized feature flags or library items
2626
msrv_aliases! {
27-
1,97,0 { ISOLATE_LOWEST_ONE }
27+
1,97,0 { ISOLATE_LOWEST_ONE, BIT_WIDTH }
2828
1,93,0 { VEC_DEQUE_POP_BACK_IF, VEC_DEQUE_POP_FRONT_IF }
2929
1,91,0 { DURATION_FROM_MINUTES_HOURS }
3030
1,88,0 { LET_CHAINS, AS_CHUNKS }

clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ generate! {
4040
AsyncReadExt,
4141
AsyncWriteExt,
4242
BACKSLASH_SINGLE_QUOTE: r"\'",
43+
BITS,
4344
BTreeEntry,
4445
BTreeSet,
4546
Binary,

tests/ui/manual_bit_width.fixed

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#![warn(clippy::manual_bit_width)]
2+
3+
use core::num::{self, NonZero, NonZeroI32, NonZeroU32};
4+
5+
fn main() {
6+
// `T::BITS - x.leading_zeros()`
7+
// unsigned
8+
let w: u8 = 5;
9+
let _ = w.bit_width(); //~ manual_bit_width
10+
let w: u16 = 5;
11+
let _ = w.bit_width(); //~ manual_bit_width
12+
let w: u32 = 5;
13+
let _ = w.bit_width(); //~ manual_bit_width
14+
let w: u64 = 5;
15+
let _ = w.bit_width(); //~ manual_bit_width
16+
let w: usize = 5;
17+
let _ = w.bit_width(); //~ manual_bit_width
18+
19+
// signed
20+
let x: i8 = -5;
21+
let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width
22+
let x: i16 = -5;
23+
let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width
24+
let x: i32 = -5;
25+
let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width
26+
let x: i64 = -5;
27+
let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width
28+
let x: isize = -5;
29+
let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width
30+
31+
// `NonZero::<T>::BITS - x.leading_zeros()`
32+
// unsigned
33+
let y = NonZero::<u8>::new(5).unwrap();
34+
let _ = y.bit_width().get(); //~ manual_bit_width
35+
let y = NonZero::<u16>::new(5).unwrap();
36+
let _ = y.bit_width().get(); //~ manual_bit_width
37+
let y = NonZero::<u32>::new(5).unwrap();
38+
let _ = y.bit_width().get(); //~ manual_bit_width
39+
let y = NonZero::<u64>::new(5).unwrap();
40+
let _ = y.bit_width().get(); //~ manual_bit_width
41+
let y = NonZero::<usize>::new(5).unwrap();
42+
let _ = y.bit_width().get(); //~ manual_bit_width
43+
44+
// signed
45+
let z = NonZero::<i8>::new(-5).unwrap();
46+
let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width
47+
let z = NonZero::<i16>::new(-5).unwrap();
48+
let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width
49+
let z = NonZero::<i32>::new(-5).unwrap();
50+
let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width
51+
let z = NonZero::<i64>::new(-5).unwrap();
52+
let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width
53+
let z = NonZero::<isize>::new(-5).unwrap();
54+
let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width
55+
56+
// negative cases.
57+
// left expression is a literal
58+
let z: u32 = 1_000_000 - x.leading_zeros();
59+
}

tests/ui/manual_bit_width.rs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
#![warn(clippy::manual_bit_width)]
2+
3+
use core::num::{self, NonZero, NonZeroI32, NonZeroU32};
4+
5+
fn main() {
6+
// `T::BITS - x.leading_zeros()`
7+
// unsigned
8+
let w: u8 = 5;
9+
let _ = u8::BITS - w.leading_zeros(); //~ manual_bit_width
10+
let w: u16 = 5;
11+
let _ = u16::BITS - w.leading_zeros(); //~ manual_bit_width
12+
let w: u32 = 5;
13+
let _ = u32::BITS - w.leading_zeros(); //~ manual_bit_width
14+
let w: u64 = 5;
15+
let _ = u64::BITS - w.leading_zeros(); //~ manual_bit_width
16+
let w: usize = 5;
17+
let _ = usize::BITS - w.leading_zeros(); //~ manual_bit_width
18+
19+
// signed
20+
let x: i8 = -5;
21+
let _ = i8::BITS - x.leading_zeros(); //~ manual_bit_width
22+
let x: i16 = -5;
23+
let _ = i16::BITS - x.leading_zeros(); //~ manual_bit_width
24+
let x: i32 = -5;
25+
let _ = i32::BITS - x.leading_zeros(); //~ manual_bit_width
26+
let x: i64 = -5;
27+
let _ = i64::BITS - x.leading_zeros(); //~ manual_bit_width
28+
let x: isize = -5;
29+
let _ = isize::BITS - x.leading_zeros(); //~ manual_bit_width
30+
31+
// `NonZero::<T>::BITS - x.leading_zeros()`
32+
// unsigned
33+
let y = NonZero::<u8>::new(5).unwrap();
34+
let _ = NonZero::<u8>::BITS - y.leading_zeros(); //~ manual_bit_width
35+
let y = NonZero::<u16>::new(5).unwrap();
36+
let _ = NonZero::<u16>::BITS - y.leading_zeros(); //~ manual_bit_width
37+
let y = NonZero::<u32>::new(5).unwrap();
38+
let _ = NonZeroU32::BITS - y.leading_zeros(); //~ manual_bit_width
39+
let y = NonZero::<u64>::new(5).unwrap();
40+
let _ = NonZero::<u64>::BITS - y.leading_zeros(); //~ manual_bit_width
41+
let y = NonZero::<usize>::new(5).unwrap();
42+
let _ = num::NonZero::<usize>::BITS - y.leading_zeros(); //~ manual_bit_width
43+
44+
// signed
45+
let z = NonZero::<i8>::new(-5).unwrap();
46+
let _ = NonZero::<i8>::BITS - z.leading_zeros(); //~ manual_bit_width
47+
let z = NonZero::<i16>::new(-5).unwrap();
48+
let _ = NonZero::<i16>::BITS - z.leading_zeros(); //~ manual_bit_width
49+
let z = NonZero::<i32>::new(-5).unwrap();
50+
let _ = NonZeroI32::BITS - z.leading_zeros(); //~ manual_bit_width
51+
let z = NonZero::<i64>::new(-5).unwrap();
52+
let _ = NonZero::<i64>::BITS - z.leading_zeros(); //~ manual_bit_width
53+
let z = NonZero::<isize>::new(-5).unwrap();
54+
let _ = num::NonZero::<isize>::BITS - z.leading_zeros(); //~ manual_bit_width
55+
56+
// negative cases.
57+
// left expression is a literal
58+
let z: u32 = 1_000_000 - x.leading_zeros();
59+
}

0 commit comments

Comments
 (0)