Skip to content

Commit 00ea73e

Browse files
committed
avoid linting possible_truncation on bit-reducing operations
1 parent df65291 commit 00ea73e

File tree

3 files changed

+101
-9
lines changed

3 files changed

+101
-9
lines changed

clippy_lints/src/casts/cast_possible_truncation.rs

+73-8
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,88 @@
1+
use clippy_utils::consts::{constant, Constant};
12
use clippy_utils::diagnostics::span_lint;
23
use clippy_utils::expr_or_init;
34
use clippy_utils::ty::is_isize_or_usize;
4-
use rustc_hir::{Expr, ExprKind};
5+
use rustc_hir::{BinOpKind, Expr, ExprKind};
56
use rustc_lint::LateContext;
67
use rustc_middle::ty::{self, FloatTy, Ty};
78

89
use super::{utils, CAST_POSSIBLE_TRUNCATION};
910

10-
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
11-
// do not lint if cast comes from a `signum` function
12-
if let ExprKind::MethodCall(path, ..) = expr_or_init(cx, cast_expr).kind {
13-
if path.ident.name.as_str() == "signum" {
14-
return;
15-
}
11+
fn constant_int(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u128> {
12+
if let Some((Constant::Int(c), _)) = constant(cx, cx.typeck_results(), expr) {
13+
Some(c)
14+
} else {
15+
None
1616
}
17+
}
1718

19+
fn get_constant_bits(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u64> {
20+
constant_int(cx, expr).map(|c| u64::from(128 - c.leading_zeros()))
21+
}
22+
23+
fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: bool) -> u64 {
24+
match expr_or_init(cx, expr).kind {
25+
ExprKind::Cast(inner, _) => apply_reductions(cx, nbits, inner, signed),
26+
ExprKind::Block(block, _) => block.expr.map_or(nbits, |e| apply_reductions(cx, nbits, e, signed)),
27+
ExprKind::Binary(op, left, right) => match op.node {
28+
BinOpKind::Div => {
29+
apply_reductions(cx, nbits, left, signed)
30+
- (if signed {
31+
0 // let's be conservative here
32+
} else {
33+
// by dividing by 1, we remove 0 bits, etc.
34+
get_constant_bits(cx, right).map_or(0, |b| b.saturating_sub(1))
35+
})
36+
},
37+
BinOpKind::Rem | BinOpKind::BitAnd => get_constant_bits(cx, right)
38+
.unwrap_or(u64::max_value())
39+
.min(apply_reductions(cx, nbits, left, signed)),
40+
BinOpKind::Shr => {
41+
apply_reductions(cx, nbits, left, signed)
42+
- constant_int(cx, right).map_or(0, |s| u64::try_from(s).expect("shift too high"))
43+
},
44+
_ => nbits,
45+
},
46+
ExprKind::MethodCall(method, _, [left, right], _) => {
47+
if signed {
48+
return nbits;
49+
}
50+
let max_bits = if method.ident.as_str() == "min" {
51+
get_constant_bits(cx, right)
52+
} else {
53+
None
54+
};
55+
apply_reductions(cx, nbits, left, signed).min(max_bits.unwrap_or(u64::max_value()))
56+
},
57+
ExprKind::MethodCall(method, _, [_, lo, hi], _) => {
58+
if method.ident.as_str() == "clamp" {
59+
//FIXME: make this a diagnostic item
60+
if let (Some(lo_bits), Some(hi_bits)) = (get_constant_bits(cx, lo), get_constant_bits(cx, hi)) {
61+
return lo_bits.max(hi_bits);
62+
}
63+
}
64+
nbits
65+
},
66+
ExprKind::MethodCall(method, _, [_value], _) => {
67+
if method.ident.name.as_str() == "signum" {
68+
0 // do not lint if cast comes from a `signum` function
69+
} else {
70+
nbits
71+
}
72+
},
73+
_ => nbits,
74+
}
75+
}
76+
77+
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, cast_expr: &Expr<'_>, cast_from: Ty<'_>, cast_to: Ty<'_>) {
1878
let msg = match (cast_from.is_integral(), cast_to.is_integral()) {
1979
(true, true) => {
20-
let from_nbits = utils::int_ty_to_nbits(cast_from, cx.tcx);
80+
let from_nbits = apply_reductions(
81+
cx,
82+
utils::int_ty_to_nbits(cast_from, cx.tcx),
83+
cast_expr,
84+
cast_from.is_signed(),
85+
);
2186
let to_nbits = utils::int_ty_to_nbits(cast_to, cx.tcx);
2287

2388
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {

tests/ui/cast.rs

+15
Original file line numberDiff line numberDiff line change
@@ -100,4 +100,19 @@ fn main() {
100100

101101
let s = x.signum();
102102
let _ = s as i32;
103+
104+
// Test for signed min
105+
(-99999999999i64).min(1) as i8; // should be linted because signed
106+
107+
// Test for various operations that remove enough bits for the result to fit
108+
(999999u64 & 1) as u8;
109+
(999999u64 % 15) as u8;
110+
(999999u64 / 0x1_0000_0000_0000) as u16;
111+
({ 999999u64 >> 56 }) as u8;
112+
({
113+
let x = 999999u64;
114+
x.min(1)
115+
}) as u8;
116+
999999u64.clamp(0, 255) as u8;
117+
999999u64.clamp(0, 256) as u8; // should still be linted
103118
}

tests/ui/cast.stderr

+13-1
Original file line numberDiff line numberDiff line change
@@ -138,5 +138,17 @@ error: casting `isize` to `usize` may lose the sign of the value
138138
LL | -1isize as usize;
139139
| ^^^^^^^^^^^^^^^^
140140

141-
error: aborting due to 22 previous errors
141+
error: casting `i64` to `i8` may truncate the value
142+
--> $DIR/cast.rs:105:5
143+
|
144+
LL | (-99999999999i64).min(1) as i8; // should be linted because signed
145+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
146+
147+
error: casting `u64` to `u8` may truncate the value
148+
--> $DIR/cast.rs:117:5
149+
|
150+
LL | 999999u64.clamp(0, 256) as u8; // should still be linted
151+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
152+
153+
error: aborting due to 24 previous errors
142154

0 commit comments

Comments
 (0)