From da94fb29224e6e45dac79d7d8e17fb64e90ec4c7 Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Wed, 13 Oct 2021 13:45:53 +0200 Subject: [PATCH] avoid linting `possible_truncation` on bit-reducing operations --- .../src/casts/cast_possible_truncation.rs | 85 +++++++++++++++++-- tests/ui/cast.rs | 15 ++++ tests/ui/cast.stderr | 14 ++- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 2ae7d16e00bb..ea60513dcaff 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -1,23 +1,92 @@ +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_hir::{Expr, ExprKind}; +use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::{self, FloatTy, Ty}; use super::{utils, CAST_POSSIBLE_TRUNCATION}; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) { - // do not lint if cast comes from a `signum` function - if let ExprKind::MethodCall(path, ..) = expr_or_init(cx, cast_expr).kind { - if path.ident.name.as_str() == "signum" { - return; - } +fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) { + Some(c) + } else { + None + } +} + +fn get_constant_bits(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option { + constant_int(cx, expr).map(|c| u64::from(128 - c.leading_zeros())) +} + +fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: bool) -> u64 { + match expr_or_init(cx, expr).kind { + ExprKind::Cast(inner, _) => apply_reductions(cx, nbits, inner, signed), + ExprKind::Block(block, _) => block.expr.map_or(nbits, |e| apply_reductions(cx, nbits, e, signed)), + ExprKind::Binary(op, left, right) => match op.node { + BinOpKind::Div => { + apply_reductions(cx, nbits, left, signed) + - (if signed { + 0 // let's be conservative here + } else { + // by dividing by 1, we remove 0 bits, etc. + get_constant_bits(cx, right).map_or(0, |b| b.saturating_sub(1)) + }) + }, + BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right) + .unwrap_or(u64::max_value()) + .min(apply_reductions(cx, nbits, left, signed)), + BinOpKind::Shr => { + apply_reductions(cx, nbits, left, signed) + - constant_int(cx, right).map_or(0, |s| u64::try_from(s).expect("shift too high")) + }, + _ => nbits, + }, + ExprKind::MethodCall(method, _, [left, right], _) => { + if signed { + return nbits; + } + let max_bits = if method.ident.as_str() == "max" { + get_constant_bits(cx, right) + } else { + None + }; + apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value())) + }, + ExprKind::MethodCall(method, _, [value, lo, hi], _) => { + let reduced_bits = apply_reductions(cx, nbits, value, signed); + if method.ident.as_str() == "clamp" { + //TODO: make this a diagnostic item + if let (Some(lo_bits), Some(hi_bits)) = (get_constant_bits(cx, lo), get_constant_bits(cx, hi)) { + reduced_bits.min(lo_bits.max(hi_bits)) + } else { + reduced_bits + } + } else { + reduced_bits + } + }, + ExprKind::MethodCall(method, _, [_value], _) => { + if method.ident.name.as_str() == "signum" { + 0 // do not lint if cast comes from a `signum` function + } else { + nbits + } + }, + _ => nbits, } +} +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 from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx); + let from_nbits = apply_reductions( + cx, + utils::int_ty_to_nbits(cast_from, cx.tcx), + cast_expr, + cast_from.is_signed(), + ); let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx); let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) { diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index adc95e63ae56..f31641da34ce 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -100,4 +100,19 @@ fn main() { let s = x.signum(); let _ = s as i32; + + // Test for signed max + (-99999999999i64).max(1) as i8; // should be linted because signed + + // Test for various operations that remove enough bits for the result to fit + (999999u64 & 1) as u8; + (999999u64 % 15) as u8; + (999999u64 / 0x1_0000_0000_0000) as u16; + ({ 999999u64 >> 56 }) as u8; + ({ + let x = 999999u64; + x.max(1) + }) as u8; + 999999u64.clamp(0, 255) as u8; + 999999u64.clamp(0, 256) as u8; // should still be linted } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 4c66d7364948..fb65003cdc6b 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -138,5 +138,17 @@ error: casting `isize` to `usize` may lose the sign of the value LL | -1isize as usize; | ^^^^^^^^^^^^^^^^ -error: aborting due to 22 previous errors +error: casting `i64` to `i8` may truncate the value + --> $DIR/cast.rs:105:5 + | +LL | (-99999999999i64).max(1) as i8; // should be linted because signed + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: casting `u64` to `u8` may truncate the value + --> $DIR/cast.rs:117:5 + | +LL | 999999u64.clamp(0, 256) as u8; // should still be linted + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 24 previous errors