From ece4d65a9c7bfede9d47d4003bd50d275e109f81 Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Mon, 31 Jan 2022 21:22:47 -0500 Subject: [PATCH] New lint `cast_enum_truncation` --- CHANGELOG.md | 1 + .../src/casts/cast_possible_truncation.rs | 65 +++++++++---- clippy_lints/src/casts/mod.rs | 20 ++++ clippy_lints/src/casts/utils.rs | 96 +++++++++++++------ clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/lib.rs | 1 + tests/ui/cast.rs | 21 ++++ tests/ui/cast.stderr | 28 +++++- 10 files changed, 183 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9548366daf9a..5c31998644da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2897,6 +2897,7 @@ Released 2018-09-13 [`bytes_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#bytes_nth [`cargo_common_metadata`]: https://rust-lang.github.io/rust-clippy/master/index.html#cargo_common_metadata [`case_sensitive_file_extension_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#case_sensitive_file_extension_comparisons +[`cast_enum_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_enum_truncation [`cast_lossless`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_lossless [`cast_possible_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_truncation [`cast_possible_wrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#cast_possible_wrap diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 2384e4a37483..6c938062c197 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -2,12 +2,14 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint; use clippy_utils::expr_or_init; use clippy_utils::ty::is_isize_or_usize; +use rustc_ast::ast; +use rustc_attr::IntType; use rustc_hir::def::{DefKind, Res}; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_middle::ty::{self, FloatTy, Ty}; +use rustc_middle::ty::{self, FloatTy, Ty, VariantDiscr}; -use super::{utils, CAST_POSSIBLE_TRUNCATION}; +use super::{utils, CAST_ENUM_TRUNCATION, CAST_POSSIBLE_TRUNCATION}; fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) { @@ -110,27 +112,54 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, }, (ty::Adt(def, _), true) if def.is_enum() => { - if let ExprKind::Path(p) = &cast_expr.kind - && let Res::Def(DefKind::Ctor(..), _) = cx.qpath_res(p, cast_expr.hir_id) + let (from_nbits, variant) = if let ExprKind::Path(p) = &cast_expr.kind + && let Res::Def(DefKind::Ctor(..), id) = cx.qpath_res(p, cast_expr.hir_id) { - return - } - - let from_nbits = utils::enum_ty_to_nbits(def, cx.tcx); + let i = def.variant_index_with_ctor_id(id); + let variant = &def.variants[i]; + let nbits: u64 = match variant.discr { + VariantDiscr::Explicit(id) => utils::read_explicit_enum_value(cx.tcx, id).unwrap().nbits(), + VariantDiscr::Relative(x) => { + match def.variants[(i.as_usize() - x as usize).into()].discr { + VariantDiscr::Explicit(id) => { + utils::read_explicit_enum_value(cx.tcx, id).unwrap().add(x).nbits() + } + VariantDiscr::Relative(_) => (32 - x.leading_zeros()).into(), + } + } + }; + (nbits, Some(variant)) + } else { + (utils::enum_ty_to_nbits(def, cx.tcx), None) + }; let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx); - let suffix = if is_isize_or_usize(cast_to) { - if from_nbits > 32 { - " on targets with 32-bit wide pointers" - } else { - return; - } - } else if to_nbits < from_nbits { - "" - } else { - return; + let cast_from_ptr_size = def.repr.int.map_or(true, |ty| { + matches!( + ty, + IntType::SignedInt(ast::IntTy::Isize) | IntType::UnsignedInt(ast::UintTy::Usize) + ) + }); + let suffix = match (cast_from_ptr_size, is_isize_or_usize(cast_to)) { + (false, false) if from_nbits > to_nbits => "", + (true, false) if from_nbits > to_nbits => "", + (false, true) if from_nbits > 64 => "", + (false, true) if from_nbits > 32 => " on targets with 32-bit wide pointers", + _ => return, }; + if let Some(variant) = variant { + span_lint( + cx, + CAST_ENUM_TRUNCATION, + expr.span, + &format!( + "casting `{}::{}` to `{}` will truncate the value{}", + cast_from, variant.name, cast_to, suffix, + ), + ); + return; + } format!( "casting `{}` to `{}` may truncate the value{}", cast_from, cast_to, suffix, diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index 17a25f212bb0..49b0196d6f8a 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -390,6 +390,25 @@ declare_clippy_lint! { "casting using `as` from and to raw pointers that doesn't change its mutability, where `pointer::cast` could take the place of `as`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for casts from an enum type to an integral type which will definitely truncate the + /// value. + /// + /// ### Why is this bad? + /// The resulting integral value will not match the value of the variant it came from. + /// + /// ### Example + /// ```rust + /// enum E { X = 256 }; + /// let _ = E::X as u8; + /// ``` + #[clippy::version = "1.60.0"] + pub CAST_ENUM_TRUNCATION, + suspicious, + "casts from an enum type to an integral type which will truncate the value" +} + pub struct Casts { msrv: Option, } @@ -415,6 +434,7 @@ impl_lint_pass!(Casts => [ FN_TO_NUMERIC_CAST_WITH_TRUNCATION, CHAR_LIT_AS_U8, PTR_AS_PTR, + CAST_ENUM_TRUNCATION, ]); impl<'tcx> LateLintPass<'tcx> for Casts { diff --git a/clippy_lints/src/casts/utils.rs b/clippy_lints/src/casts/utils.rs index d41e745d7335..fab0c194b375 100644 --- a/clippy_lints/src/casts/utils.rs +++ b/clippy_lints/src/casts/utils.rs @@ -1,5 +1,6 @@ use rustc_middle::mir::interpret::{ConstValue, Scalar}; use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TyCtxt, UintTy, VariantDiscr}; +use rustc_span::def_id::DefId; use rustc_target::abi::Size; /// Returns the size in bits of an integral type. @@ -26,48 +27,83 @@ pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 { } } +pub(super) enum EnumValue { + Unsigned(u128), + Signed(i128), +} +impl EnumValue { + pub(super) fn add(self, n: u32) -> Self { + match self { + Self::Unsigned(x) => Self::Unsigned(x + u128::from(n)), + Self::Signed(x) => Self::Signed(x + i128::from(n)), + } + } + + pub(super) fn nbits(self) -> u64 { + match self { + Self::Unsigned(x) => 128 - x.leading_zeros(), + Self::Signed(x) if x < 0 => 128 - (-(x + 1)).leading_zeros() + 1, + Self::Signed(x) => 128 - x.leading_zeros(), + } + .into() + } +} + +#[allow(clippy::cast_possible_truncation, clippy::cast_possible_wrap)] +pub(super) fn read_explicit_enum_value(tcx: TyCtxt<'_>, id: DefId) -> Option { + if let Ok(ConstValue::Scalar(Scalar::Int(value))) = tcx.const_eval_poly(id) { + match tcx.type_of(id).kind() { + ty::Int(_) => Some(EnumValue::Signed(match value.size().bytes() { + 1 => i128::from(value.assert_bits(Size::from_bytes(1)) as u8 as i8), + 2 => i128::from(value.assert_bits(Size::from_bytes(2)) as u16 as i16), + 4 => i128::from(value.assert_bits(Size::from_bytes(4)) as u32 as i32), + 8 => i128::from(value.assert_bits(Size::from_bytes(8)) as u64 as i64), + 16 => value.assert_bits(Size::from_bytes(16)) as i128, + _ => return None, + })), + ty::Uint(_) => Some(EnumValue::Unsigned(match value.size().bytes() { + 1 => value.assert_bits(Size::from_bytes(1)), + 2 => value.assert_bits(Size::from_bytes(2)), + 4 => value.assert_bits(Size::from_bytes(4)), + 8 => value.assert_bits(Size::from_bytes(8)), + 16 => value.assert_bits(Size::from_bytes(16)), + _ => return None, + })), + _ => None, + } + } else { + None + } +} + pub(super) fn enum_ty_to_nbits(adt: &AdtDef, tcx: TyCtxt<'_>) -> u64 { let mut explicit = 0i128; let (start, end) = adt .variants .iter() - .fold((i128::MAX, i128::MIN), |(start, end), variant| match variant.discr { + .fold((0, i128::MIN), |(start, end), variant| match variant.discr { VariantDiscr::Relative(x) => match explicit.checked_add(i128::from(x)) { Some(x) => (start, end.max(x)), None => (i128::MIN, end), }, - VariantDiscr::Explicit(id) => { - let ty = tcx.type_of(id); - if let Ok(ConstValue::Scalar(Scalar::Int(value))) = tcx.const_eval_poly(id) { - #[allow(clippy::cast_possible_truncation, clippy::cast_possible_wrap)] - let value = match (value.size().bytes(), ty.kind()) { - (1, ty::Int(_)) => i128::from(value.assert_bits(Size::from_bytes(1)) as u8 as i8), - (1, ty::Uint(_)) => i128::from(value.assert_bits(Size::from_bytes(1)) as u8), - (2, ty::Int(_)) => i128::from(value.assert_bits(Size::from_bytes(2)) as u16 as i16), - (2, ty::Uint(_)) => i128::from(value.assert_bits(Size::from_bytes(2)) as u16), - (4, ty::Int(_)) => i128::from(value.assert_bits(Size::from_bytes(4)) as u32 as i32), - (4, ty::Uint(_)) => i128::from(value.assert_bits(Size::from_bytes(4)) as u32), - (8, ty::Int(_)) => i128::from(value.assert_bits(Size::from_bytes(8)) as u64 as i64), - (8, ty::Uint(_)) => i128::from(value.assert_bits(Size::from_bytes(8)) as u64), - (16, ty::Int(_)) => value.assert_bits(Size::from_bytes(16)) as i128, - (16, ty::Uint(_)) => match i128::try_from(value.assert_bits(Size::from_bytes(16))) { - Ok(x) => x, - // Requires 128 bits - Err(_) => return (i128::MIN, end), - }, - // Shouldn't happen if compilation was successful - _ => return (start, end), - }; - explicit = value; - (start.min(value), end.max(value)) - } else { - // Shouldn't happen if compilation was successful - (start, end) - } + VariantDiscr::Explicit(id) => match read_explicit_enum_value(tcx, id) { + Some(EnumValue::Signed(x)) => { + explicit = x; + (start.min(x), end.max(x)) + }, + Some(EnumValue::Unsigned(x)) => match i128::try_from(x) { + Ok(x) => { + explicit = x; + (start, end.max(x)) + }, + Err(_) => (i128::MIN, end), + }, + None => (start, end), }, }); - if start >= end { + if start > end { + // No variants. 0 } else { let neg_bits = if start < 0 { diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4721b7f2b472..d22c94ffdae0 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -21,6 +21,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(bool_assert_comparison::BOOL_ASSERT_COMPARISON), LintId::of(booleans::LOGIC_BUG), LintId::of(booleans::NONMINIMAL_BOOL), + LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(casts::CAST_REF_TO_MUT), LintId::of(casts::CHAR_LIT_AS_U8), LintId::of(casts::FN_TO_NUMERIC_CAST), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index e5e1c052c15e..7722ab307b04 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -63,6 +63,7 @@ store.register_lints(&[ bytecount::NAIVE_BYTECOUNT, cargo_common_metadata::CARGO_COMMON_METADATA, case_sensitive_file_extension_comparisons::CASE_SENSITIVE_FILE_EXTENSION_COMPARISONS, + casts::CAST_ENUM_TRUNCATION, casts::CAST_LOSSLESS, casts::CAST_POSSIBLE_TRUNCATION, casts::CAST_POSSIBLE_WRAP, diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs index 10f8ae4b7f7f..5a5ab4169ee4 100644 --- a/clippy_lints/src/lib.register_suspicious.rs +++ b/clippy_lints/src/lib.register_suspicious.rs @@ -5,6 +5,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![ LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP), LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS), + LintId::of(casts::CAST_ENUM_TRUNCATION), LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE), LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS), LintId::of(formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0b07726519ec..fd00e2be2b88 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -23,6 +23,7 @@ // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_ast; extern crate rustc_ast_pretty; +extern crate rustc_attr; extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_errors; diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 371bf292f747..2e31ad3172ee 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -139,7 +139,9 @@ fn main() { impl E2 { fn test(self) { let _ = self as u8; + let _ = Self::B as u8; let _ = self as i16; // Don't lint. `255..=256` fits in i16 + let _ = Self::A as u8; // Don't lint. } } @@ -174,7 +176,9 @@ fn main() { impl E5 { fn test(self) { let _ = self as i8; + let _ = Self::A as i8; let _ = self as i16; // Don't lint. `-129..=127` fits in i16 + let _ = Self::B as u8; // Don't lint. } } @@ -189,6 +193,7 @@ fn main() { let _ = self as i16; let _ = Self::A as u16; // Don't lint. `2^16-1` fits in u16 let _ = self as u32; // Don't lint. `2^16-1..=2^16` fits in u32 + let _ = Self::A as u16; // Don't lint. } } @@ -201,6 +206,7 @@ fn main() { impl E7 { fn test(self) { let _ = self as usize; + let _ = Self::A as usize; // Don't lint. let _ = self as u64; // Don't lint. `2^32-1..=2^32` fits in u64 } } @@ -227,7 +233,22 @@ fn main() { } impl E9 { fn test(self) { + let _ = Self::A as u8; // Don't lint. let _ = self as u128; // Don't lint. `0..=2^128-1` fits in u128 } } + + #[derive(Clone, Copy)] + #[repr(usize)] + enum E10 { + A, + B = u32::MAX as usize, + } + impl E10 { + fn test(self) { + let _ = self as u16; + let _ = Self::B as u32; // Don't lint. + let _ = self as u64; // Don't lint. + } + } } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 0fc66f739811..7a68c0984f14 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -156,23 +156,43 @@ error: casting `main::E2` to `u8` may truncate the value LL | let _ = self as u8; | ^^^^^^^^^^ +error: casting `main::E2::B` to `u8` will truncate the value + --> $DIR/cast.rs:142:21 + | +LL | let _ = Self::B as u8; + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::cast-enum-truncation` implied by `-D warnings` + error: casting `main::E5` to `i8` may truncate the value - --> $DIR/cast.rs:176:21 + --> $DIR/cast.rs:178:21 | LL | let _ = self as i8; | ^^^^^^^^^^ +error: casting `main::E5::A` to `i8` will truncate the value + --> $DIR/cast.rs:179:21 + | +LL | let _ = Self::A as i8; + | ^^^^^^^^^^^^^ + error: casting `main::E6` to `i16` may truncate the value - --> $DIR/cast.rs:189:21 + --> $DIR/cast.rs:193:21 | LL | let _ = self as i16; | ^^^^^^^^^^^ error: casting `main::E7` to `usize` may truncate the value on targets with 32-bit wide pointers - --> $DIR/cast.rs:203:21 + --> $DIR/cast.rs:208:21 | LL | let _ = self as usize; | ^^^^^^^^^^^^^ -error: aborting due to 28 previous errors +error: casting `main::E10` to `u16` may truncate the value + --> $DIR/cast.rs:249:21 + | +LL | let _ = self as u16; + | ^^^^^^^^^^^ + +error: aborting due to 31 previous errors