Skip to content

Commit

Permalink
avoid linting possible_truncation on bit-reducing operations
Browse files Browse the repository at this point in the history
  • Loading branch information
llogiq committed Oct 29, 2021
1 parent df65291 commit 00ea73e
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 9 deletions.
81 changes: 73 additions & 8 deletions clippy_lints/src/casts/cast_possible_truncation.rs
@@ -1,23 +1,88 @@
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<u128> {
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<u64> {
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() == "min" {
get_constant_bits(cx, right)
} else {
None
};
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
},
ExprKind::MethodCall(method, _, [_, lo, hi], _) => {
if method.ident.as_str() == "clamp" {
//FIXME: make this a diagnostic item
if let (Some(lo_bits), Some(hi_bits)) = (get_constant_bits(cx, lo), get_constant_bits(cx, hi)) {
return lo_bits.max(hi_bits);
}
}
nbits
},
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)) {
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/cast.rs
Expand Up @@ -100,4 +100,19 @@ fn main() {

let s = x.signum();
let _ = s as i32;

// Test for signed min
(-99999999999i64).min(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.min(1)
}) as u8;
999999u64.clamp(0, 255) as u8;
999999u64.clamp(0, 256) as u8; // should still be linted
}
14 changes: 13 additions & 1 deletion tests/ui/cast.stderr
Expand Up @@ -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).min(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

0 comments on commit 00ea73e

Please sign in to comment.