-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Lint bit width #16902
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
Merged
Merged
Lint bit width #16902
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| use clippy_config::Conf; | ||
| use clippy_utils::diagnostics::span_lint_and_then; | ||
| use clippy_utils::msrvs::{self, Msrv}; | ||
| use clippy_utils::source::snippet_with_context; | ||
| use clippy_utils::{is_from_proc_macro, sym}; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; | ||
| use rustc_lint::{LateContext, LateLintPass, LintContext}; | ||
| use rustc_middle::ty::{self, Ty}; | ||
| use rustc_session::impl_lint_pass; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Checks for usage of `T::BITS - x.leading_zeros()` when `x.bit_width()` is available. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// Manual reimplementations of `bit_width` increase code complexity for little benefit. | ||
| /// | ||
| /// ### Example | ||
| /// ```no_run | ||
| /// let x: u32 = 5; | ||
| /// let bit_width = u32::BITS - x.leading_zeros(); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```no_run | ||
| /// let x: u32 = 5; | ||
| /// let bit_width = x.bit_width(); | ||
| /// ``` | ||
| #[clippy::version = "1.98.0"] | ||
| pub MANUAL_BIT_WIDTH, | ||
| pedantic, | ||
| "manually reimplementing `bit_width`" | ||
| } | ||
|
|
||
| declare_clippy_lint! { | ||
| /// ### What it does | ||
| /// Checks for usage of `T::BITS - x.leading_zeros()` where T and x are of different types. | ||
| /// | ||
| /// ### Why is this bad? | ||
| /// Substracting `leading_zeros` from the number of bits of another type might be | ||
| /// a buggy implementation of the `bit_width` method. | ||
| /// | ||
| /// ### Example | ||
| /// ```no_run | ||
| /// let x: u64 = 5; | ||
| /// let bit_width = u32::BITS - x.leading_zeros(); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```no_run | ||
| /// let x: u64 = 5; | ||
| /// let bit_width = x.bit_width(); | ||
| /// ``` | ||
| #[clippy::version = "1.98.0"] | ||
| pub MISMATCHED_BIT_WIDTH_TYPE, | ||
| suspicious, | ||
| "type mismatch in bit width calculation" | ||
| } | ||
|
|
||
| impl_lint_pass!(ManualBitWidth => [MANUAL_BIT_WIDTH, MISMATCHED_BIT_WIDTH_TYPE]); | ||
|
|
||
| #[derive(Clone, Copy, PartialEq)] | ||
| enum IntKind<'a> { | ||
| Int(ty::IntTy), | ||
| Uint(ty::UintTy), | ||
| // NOTE: in the following two variants, the inner `Ty` stores the entire `NonZero<T>` | ||
| // and not just `T`. This is so that we can print it in the suggestion. | ||
| NonZero(Ty<'a>), | ||
| NonZeroU(Ty<'a>), | ||
| } | ||
|
|
||
| impl IntKind<'_> { | ||
| fn inner_ty(self) -> String { | ||
| match self { | ||
| Self::Int(ty) => ty.name_str().to_string(), | ||
| Self::Uint(ty) => ty.name_str().to_string(), | ||
| Self::NonZero(ty) | Self::NonZeroU(ty) => ty.to_string(), | ||
| } | ||
| } | ||
|
|
||
| fn suggestion(&self) -> &'static str { | ||
| match self { | ||
| Self::Int(_) => ".cast_unsigned().bit_width()", | ||
| Self::Uint(_) => ".bit_width()", | ||
| Self::NonZero(_) => ".cast_unsigned().bit_width().get()", | ||
| Self::NonZeroU(_) => ".bit_width().get()", | ||
| } | ||
| } | ||
| } | ||
|
|
||
| pub struct ManualBitWidth { | ||
| msrv: Msrv, | ||
| } | ||
|
|
||
| impl ManualBitWidth { | ||
| pub fn new(conf: &Conf) -> Self { | ||
| Self { msrv: conf.msrv } | ||
| } | ||
| } | ||
|
|
||
| impl LateLintPass<'_> for ManualBitWidth { | ||
| fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { | ||
| if expr.span.in_external_macro(cx.sess().source_map()) { | ||
| return; | ||
| } | ||
|
|
||
| match expr.kind { | ||
| // `T::BITS - n.leading_zeros()` | ||
| ExprKind::Binary(op, left, right) | ||
| if op.node == BinOpKind::Sub | ||
| && let ExprKind::MethodCall(leading_zeros, recv, [], _) = right.kind | ||
| && leading_zeros.ident.name == sym::leading_zeros | ||
| && let ExprKind::Path(QPath::TypeRelative(hir_ty, segment)) = left.kind | ||
| && segment.ident.name == sym::BITS | ||
| && let right_ty = cx.typeck_results().expr_ty(recv) | ||
| && let Some(right_int_kind) = get_int_kind(cx, right_ty) | ||
| && let left_ty = cx.typeck_results().node_type(hir_ty.hir_id) | ||
| && let Some(left_int_kind) = get_int_kind(cx, left_ty) | ||
| && self.msrv.meets(cx, msrvs::BIT_WIDTH) | ||
| && left.span.eq_ctxt(right.span) | ||
| && !is_from_proc_macro(cx, expr) => | ||
| { | ||
| if left_int_kind == right_int_kind { | ||
| // manual implementation of bit_width | ||
| emit_manual_bit_width(cx, recv, expr, right_int_kind); | ||
| } else { | ||
| // mismatched calling types | ||
| emit_type_mismatch(cx, recv, expr, right_int_kind); | ||
| } | ||
| }, | ||
| _ => {}, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fn get_int_kind<'a>(cx: &LateContext<'a>, ty: Ty<'a>) -> Option<IntKind<'a>> { | ||
| match ty.kind() { | ||
| // int::BITS or uint::BITS | ||
| ty::Int(int_ty) => Some(IntKind::Int(*int_ty)), | ||
| ty::Uint(uint_ty) => Some(IntKind::Uint(*uint_ty)), | ||
| // NonZero::<int/uint>::BITS | ||
| ty::Adt(adt, args) if cx.tcx.is_diagnostic_item(sym::NonZero, adt.did()) => { | ||
| let arg = args.type_at(0); | ||
| match arg.kind() { | ||
| ty::Int(_) => Some(IntKind::NonZero(ty)), | ||
| ty::Uint(_) => Some(IntKind::NonZeroU(ty)), | ||
| _ => None, | ||
| } | ||
| }, | ||
| _ => None, | ||
| } | ||
| } | ||
|
|
||
| fn emit_manual_bit_width(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>, ty_kind: IntKind<'_>) { | ||
| span_lint_and_then( | ||
| cx, | ||
| MANUAL_BIT_WIDTH, | ||
| full_expr.span, | ||
| "manual implementation of `bit_width`", | ||
| |diag| { | ||
| let mut app = Applicability::MachineApplicable; | ||
| let (recv_snip, _) = snippet_with_context(cx, recv.span, full_expr.span.ctxt(), "_", &mut app); | ||
| let suggestion = ty_kind.suggestion(); | ||
|
|
||
| diag.span_suggestion_verbose(full_expr.span, "try", format!("{recv_snip}{suggestion}"), app); | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| fn emit_type_mismatch(cx: &LateContext<'_>, recv: &Expr<'_>, full_expr: &Expr<'_>, ty_kind: IntKind<'_>) { | ||
| span_lint_and_then( | ||
| cx, | ||
| MISMATCHED_BIT_WIDTH_TYPE, | ||
| full_expr.span, | ||
| "possible buggy implementation of `bit_width`", | ||
| |diag| { | ||
| diag.note("in order to calculate the bit width, `T::BITS` should match the type of the value calling `.leading_zeros()`"); | ||
|
|
||
| let mut app = Applicability::MaybeIncorrect; | ||
| let (recv_snip, _) = snippet_with_context(cx, recv.span, full_expr.span.ctxt(), "_", &mut app); | ||
| let suggestion = ty_kind.suggestion(); | ||
| let x_ty = ty_kind.inner_ty(); | ||
|
|
||
| diag.span_suggestion_verbose( | ||
| full_expr.span, | ||
| format!("if you meant to use `{x_ty}::BITS`, use"), | ||
| format!("{recv_snip}{suggestion}"), | ||
| app, | ||
| ); | ||
| }, | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| #![warn(clippy::manual_bit_width)] | ||
|
|
||
| use core::num::{self, NonZero, NonZeroI32, NonZeroU32}; | ||
|
|
||
| fn main() { | ||
| // `T::BITS - x.leading_zeros()` | ||
| // unsigned | ||
| let w: u8 = 5; | ||
| let _ = w.bit_width(); //~ manual_bit_width | ||
| let w: u16 = 5; | ||
| let _ = w.bit_width(); //~ manual_bit_width | ||
| let w: u32 = 5; | ||
| let _ = w.bit_width(); //~ manual_bit_width | ||
| let w: u64 = 5; | ||
| let _ = w.bit_width(); //~ manual_bit_width | ||
| let w: usize = 5; | ||
| let _ = w.bit_width(); //~ manual_bit_width | ||
|
|
||
| // signed | ||
| let x: i8 = -5; | ||
| let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width | ||
| let x: i16 = -5; | ||
| let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width | ||
| let x: i32 = -5; | ||
| let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width | ||
| let x: i64 = -5; | ||
| let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width | ||
| let x: isize = -5; | ||
| let _ = x.cast_unsigned().bit_width(); //~ manual_bit_width | ||
|
|
||
| // `NonZero::<T>::BITS - x.leading_zeros()` | ||
| // unsigned | ||
| let y = NonZero::<u8>::new(5).unwrap(); | ||
| let _ = y.bit_width().get(); //~ manual_bit_width | ||
| let y = NonZero::<u16>::new(5).unwrap(); | ||
| let _ = y.bit_width().get(); //~ manual_bit_width | ||
| let y = NonZero::<u32>::new(5).unwrap(); | ||
| let _ = y.bit_width().get(); //~ manual_bit_width | ||
| let y = NonZero::<u64>::new(5).unwrap(); | ||
| let _ = y.bit_width().get(); //~ manual_bit_width | ||
| let y = NonZero::<usize>::new(5).unwrap(); | ||
| let _ = y.bit_width().get(); //~ manual_bit_width | ||
|
|
||
| // signed | ||
| let z = NonZero::<i8>::new(-5).unwrap(); | ||
| let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width | ||
| let z = NonZero::<i16>::new(-5).unwrap(); | ||
| let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width | ||
| let z = NonZero::<i32>::new(-5).unwrap(); | ||
| let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width | ||
| let z = NonZero::<i64>::new(-5).unwrap(); | ||
| let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width | ||
| let z = NonZero::<isize>::new(-5).unwrap(); | ||
| let _ = z.cast_unsigned().bit_width().get(); //~ manual_bit_width | ||
|
|
||
| // negative cases. | ||
| // left expression is a literal | ||
| let z: u32 = 1_000_000 - x.leading_zeros(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| #![warn(clippy::manual_bit_width)] | ||
|
|
||
| use core::num::{self, NonZero, NonZeroI32, NonZeroU32}; | ||
|
|
||
| fn main() { | ||
|
alv-around marked this conversation as resolved.
|
||
| // `T::BITS - x.leading_zeros()` | ||
| // unsigned | ||
| let w: u8 = 5; | ||
| let _ = u8::BITS - w.leading_zeros(); //~ manual_bit_width | ||
| let w: u16 = 5; | ||
| let _ = u16::BITS - w.leading_zeros(); //~ manual_bit_width | ||
| let w: u32 = 5; | ||
| let _ = u32::BITS - w.leading_zeros(); //~ manual_bit_width | ||
| let w: u64 = 5; | ||
| let _ = u64::BITS - w.leading_zeros(); //~ manual_bit_width | ||
| let w: usize = 5; | ||
| let _ = usize::BITS - w.leading_zeros(); //~ manual_bit_width | ||
|
|
||
| // signed | ||
|
ada4a marked this conversation as resolved.
|
||
| let x: i8 = -5; | ||
| let _ = i8::BITS - x.leading_zeros(); //~ manual_bit_width | ||
| let x: i16 = -5; | ||
| let _ = i16::BITS - x.leading_zeros(); //~ manual_bit_width | ||
| let x: i32 = -5; | ||
| let _ = i32::BITS - x.leading_zeros(); //~ manual_bit_width | ||
| let x: i64 = -5; | ||
| let _ = i64::BITS - x.leading_zeros(); //~ manual_bit_width | ||
| let x: isize = -5; | ||
| let _ = isize::BITS - x.leading_zeros(); //~ manual_bit_width | ||
|
|
||
| // `NonZero::<T>::BITS - x.leading_zeros()` | ||
| // unsigned | ||
| let y = NonZero::<u8>::new(5).unwrap(); | ||
| let _ = NonZero::<u8>::BITS - y.leading_zeros(); //~ manual_bit_width | ||
| let y = NonZero::<u16>::new(5).unwrap(); | ||
| let _ = NonZero::<u16>::BITS - y.leading_zeros(); //~ manual_bit_width | ||
| let y = NonZero::<u32>::new(5).unwrap(); | ||
| let _ = NonZeroU32::BITS - y.leading_zeros(); //~ manual_bit_width | ||
| let y = NonZero::<u64>::new(5).unwrap(); | ||
| let _ = NonZero::<u64>::BITS - y.leading_zeros(); //~ manual_bit_width | ||
| let y = NonZero::<usize>::new(5).unwrap(); | ||
| let _ = num::NonZero::<usize>::BITS - y.leading_zeros(); //~ manual_bit_width | ||
|
|
||
| // signed | ||
| let z = NonZero::<i8>::new(-5).unwrap(); | ||
| let _ = NonZero::<i8>::BITS - z.leading_zeros(); //~ manual_bit_width | ||
| let z = NonZero::<i16>::new(-5).unwrap(); | ||
| let _ = NonZero::<i16>::BITS - z.leading_zeros(); //~ manual_bit_width | ||
| let z = NonZero::<i32>::new(-5).unwrap(); | ||
| let _ = NonZeroI32::BITS - z.leading_zeros(); //~ manual_bit_width | ||
| let z = NonZero::<i64>::new(-5).unwrap(); | ||
| let _ = NonZero::<i64>::BITS - z.leading_zeros(); //~ manual_bit_width | ||
| let z = NonZero::<isize>::new(-5).unwrap(); | ||
| let _ = num::NonZero::<isize>::BITS - z.leading_zeros(); //~ manual_bit_width | ||
|
|
||
| // negative cases. | ||
| // left expression is a literal | ||
| let z: u32 = 1_000_000 - x.leading_zeros(); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.