Skip to content
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

Lint enum-to-int casts with cast_possible_truncation #8381

Merged
merged 3 commits into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3068,6 +3068,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
68 changes: 56 additions & 12 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
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 clippy_utils::ty::{get_discriminant_value, 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 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 @@ -75,8 +78,8 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
}

pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
let msg = match (cast_from.is_integral(), cast_to.is_integral()) {
(true, true) => {
let msg = match (cast_from.kind(), cast_to.is_integral()) {
(ty::Int(_) | ty::Uint(_), true) => {
let from_nbits = apply_reductions(
cx,
utils::int_ty_to_nbits(cast_from, cx.tcx),
Expand Down Expand Up @@ -108,19 +111,60 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>,
)
},

(false, true) => {
format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to)
},

(_, _) => {
if matches!(cast_from.kind(), &ty::Float(FloatTy::F64))
&& matches!(cast_to.kind(), &ty::Float(FloatTy::F32))
(ty::Adt(def, _), true) if def.is_enum() => {
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)
{
"casting `f64` to `f32` may truncate the value".to_string()
let i = def.variant_index_with_ctor_id(id);
let variant = &def.variants[i];
let nbits = utils::enum_value_nbits(get_discriminant_value(cx.tcx, def, i));
(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 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,
)
},

(ty::Float(_), true) => {
format!("casting `{}` to `{}` may truncate the value", cast_from, cast_to)
},

(ty::Float(FloatTy::F64), false) if matches!(cast_to.kind(), &ty::Float(FloatTy::F32)) => {
"casting `f64` to `f32` may truncate the value".to_string()
},

_ => return,
};

span_lint(cx, CAST_POSSIBLE_TRUNCATION, expr.span, &msg);
Expand Down
23 changes: 21 additions & 2 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 Expand Up @@ -445,13 +465,12 @@ impl<'tcx> LateLintPass<'tcx> for Casts {
fn_to_numeric_cast_with_truncation::check(cx, expr, cast_expr, cast_from, cast_to);

if cast_to.is_numeric() && !in_external_macro(cx.sess(), expr.span) {
cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
if cast_from.is_numeric() {
cast_possible_truncation::check(cx, expr, cast_expr, cast_from, cast_to);
cast_possible_wrap::check(cx, expr, cast_from, cast_to);
cast_precision_loss::check(cx, expr, cast_from, cast_to);
cast_sign_loss::check(cx, expr, cast_expr, cast_from, cast_to);
}

cast_lossless::check(cx, expr, cast_expr, cast_from, cast_to, &self.msrv);
}
}
Expand Down
52 changes: 51 additions & 1 deletion clippy_lints/src/casts/utils.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_middle::ty::{self, IntTy, Ty, TyCtxt, UintTy};
use clippy_utils::ty::{read_explicit_enum_value, EnumValue};
use rustc_middle::ty::{self, AdtDef, IntTy, Ty, TyCtxt, UintTy, VariantDiscr};

/// Returns the size in bits of an integral type.
/// Will return 0 if the type is not an int or uint variant
Expand All @@ -23,3 +24,52 @@ pub(super) fn int_ty_to_nbits(typ: Ty<'_>, tcx: TyCtxt<'_>) -> u64 {
_ => 0,
}
}

pub(super) fn enum_value_nbits(value: EnumValue) -> u64 {
match value {
EnumValue::Unsigned(x) => 128 - x.leading_zeros(),
EnumValue::Signed(x) if x < 0 => 128 - (-(x + 1)).leading_zeros() + 1,
EnumValue::Signed(x) => 128 - x.leading_zeros(),
}
.into()
}

pub(super) fn enum_ty_to_nbits(adt: &AdtDef, tcx: TyCtxt<'_>) -> u64 {
let mut explicit = 0i128;
let (start, end) = adt
.variants
.iter()
.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) => 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 {
// No variants.
0
} else {
let neg_bits = if start < 0 {
128 - (-(start + 1)).leading_zeros() + 1
} else {
0
};
let pos_bits = if end > 0 { 128 - end.leading_zeros() } else { 0 };
neg_bits.max(pos_bits).into()
}
}
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 @@ -23,6 +23,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 @@ -67,6 +67,7 @@ store.register_lints(&[
cargo::REDUNDANT_FEATURE_NAMES,
cargo::WILDCARD_DEPENDENCIES,
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 @@ -7,6 +7,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
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 @@ -25,6 +25,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
59 changes: 58 additions & 1 deletion clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ use rustc_hir::def_id::DefId;
use rustc_hir::{Expr, TyKind, Unsafety};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::LateContext;
use rustc_middle::mir::interpret::{ConstValue, Scalar};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst};
use rustc_middle::ty::{
self, AdtDef, Binder, FnSig, IntTy, Predicate, PredicateKind, Ty, TyCtxt, TypeFoldable, UintTy,
self, AdtDef, Binder, FnSig, IntTy, Predicate, PredicateKind, Ty, TyCtxt, TypeFoldable, UintTy, VariantDiscr,
};
use rustc_span::symbol::Ident;
use rustc_span::{sym, Span, Symbol, DUMMY_SP};
use rustc_target::abi::{Size, VariantIdx};
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::query::normalize::AtExt;
use std::iter;
Expand Down Expand Up @@ -515,3 +517,58 @@ pub fn expr_sig<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>) -> Option<ExprFnS
}
}
}

#[derive(Clone, Copy)]
pub enum EnumValue {
Unsigned(u128),
Signed(i128),
}
impl core::ops::Add<u32> for EnumValue {
type Output = Self;
fn add(self, n: u32) -> Self::Output {
match self {
Self::Unsigned(x) => Self::Unsigned(x + u128::from(n)),
Self::Signed(x) => Self::Signed(x + i128::from(n)),
}
}
}

/// Attempts to read the given constant as though it were an an enum value.
#[allow(clippy::cast_possible_truncation, clippy::cast_possible_wrap)]
pub 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
}
}

/// Gets the value of the given variant.
pub fn get_discriminant_value(tcx: TyCtxt<'_>, adt: &'_ AdtDef, i: VariantIdx) -> EnumValue {
let variant = &adt.variants[i];
match variant.discr {
VariantDiscr::Explicit(id) => read_explicit_enum_value(tcx, id).unwrap(),
VariantDiscr::Relative(x) => match adt.variants[(i.as_usize() - x as usize).into()].discr {
VariantDiscr::Explicit(id) => read_explicit_enum_value(tcx, id).unwrap() + x,
VariantDiscr::Relative(_) => EnumValue::Unsigned(x.into()),
},
}
}
Loading