Skip to content

Commit

Permalink
New lint cast_enum_truncation
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Feb 1, 2022
1 parent 0f3bfc2 commit ece4d65
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 52 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 47 additions & 18 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<u128> {
if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) {
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions clippy_lints/src/casts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<RustcVersion>,
}
Expand All @@ -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 {
Expand Down
96 changes: 66 additions & 30 deletions clippy_lints/src/casts/utils.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<EnumValue> {
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 {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_suspicious.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
}

Expand Down Expand Up @@ -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.
}
}

Expand All @@ -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.
}
}

Expand All @@ -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
}
}
Expand All @@ -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.
}
}
}
28 changes: 24 additions & 4 deletions tests/ui/cast.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ece4d65

Please sign in to comment.