Skip to content

Commit 436f9ee

Browse files
committed
Auto merge of #12962 - tesuji:cast-truncating, r=Jarcho
cast_possible_truncation: Fix some false-positive cases Fix partially #7486 (comment). Fix #9613. changelog: [`cast_possible_truncation`]: fix some false-positive on % operator, valid constants, and size_of
2 parents 7ac242c + 3770c8f commit 436f9ee

File tree

3 files changed

+110
-6
lines changed

3 files changed

+110
-6
lines changed

clippy_lints/src/casts/cast_possible_truncation.rs

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ use clippy_utils::sugg::Sugg;
66
use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize};
77
use rustc_errors::{Applicability, Diag, SuggestionStyle};
88
use rustc_hir::def::{DefKind, Res};
9-
use rustc_hir::{BinOpKind, Expr, ExprKind};
9+
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
1010
use rustc_lint::LateContext;
11-
use rustc_middle::ty::{self, FloatTy, Ty};
11+
use rustc_middle::ty::{self, FloatTy, Ty, TyCtxt};
12+
use rustc_span::symbol::sym;
1213
use rustc_span::Span;
1314
use rustc_target::abi::IntegerType;
1415

@@ -40,8 +41,8 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
4041
get_constant_bits(cx, right).map_or(0, |b| b.saturating_sub(1))
4142
})
4243
},
43-
BinOpKind::Rem => get_constant_bits(cx, right)
44-
.unwrap_or(u64::MAX)
44+
BinOpKind::Rem => constant_int(cx, right)
45+
.map_or(u64::MAX, |c| c.next_power_of_two().trailing_zeros().into())
4546
.min(apply_reductions(cx, nbits, left, signed)),
4647
BinOpKind::BitAnd => get_constant_bits(cx, right)
4748
.unwrap_or(u64::MAX)
@@ -79,6 +80,35 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
7980
nbits
8081
}
8182
},
83+
ExprKind::Path(QPath::Resolved(
84+
None,
85+
rustc_hir::Path {
86+
res: Res::Def(DefKind::Const | DefKind::AssocConst, def_id),
87+
..
88+
},
89+
))
90+
// NOTE(@Jarcho): A constant from another crate might not have the same value
91+
// for all versions of the crate. This isn't a problem for constants in the
92+
// current crate since a crate can't compile against multiple versions of itself.
93+
if def_id.is_local() => {
94+
// `constant()` already checks if a const item is based on `cfg!`.
95+
get_constant_bits(cx, expr).unwrap_or(nbits)
96+
},
97+
// mem::size_of::<T>();
98+
ExprKind::Call(func, []) => {
99+
if let ExprKind::Path(ref func_qpath) = func.kind
100+
&& let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id()
101+
&& cx.tcx.is_diagnostic_item(sym::mem_size_of, def_id)
102+
&& let Some(ty) = cx.typeck_results().node_args(func.hir_id).types().next()
103+
&& is_valid_sizeof(cx.tcx, ty)
104+
&& let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty))
105+
{
106+
let size: u64 = layout.layout.size().bytes();
107+
(64 - size.leading_zeros()).into()
108+
} else {
109+
nbits
110+
}
111+
},
82112
_ => nbits,
83113
}
84114
}
@@ -104,7 +134,7 @@ pub(super) fn check(
104134
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
105135
(true, true) | (false, false) => (to_nbits < from_nbits, ""),
106136
(true, false) => (
107-
to_nbits <= 32,
137+
to_nbits < from_nbits && to_nbits <= 32,
108138
if to_nbits == 32 {
109139
" on targets with 64-bit wide pointers"
110140
} else {
@@ -199,3 +229,23 @@ fn offer_suggestion(
199229
SuggestionStyle::ShowAlways,
200230
);
201231
}
232+
233+
// FIXME(@tesuji): May extend this to a validator functions to include:
234+
// * some ABI-guaranteed STD types,
235+
// * some non-local crate types suggested in [PR-12962][1].
236+
// [1]: https://github.com/rust-lang/rust-clippy/pull/12962#discussion_r1661500351
237+
fn is_valid_sizeof<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
238+
if ty.is_primitive_ty() || ty.is_any_ptr() || ty.is_box() || ty.is_slice() {
239+
return true;
240+
}
241+
if let ty::Adt(def, args) = ty.kind()
242+
&& def.did().is_local()
243+
{
244+
def.all_fields().all(|field| {
245+
let ty = field.ty(tcx, args);
246+
is_valid_sizeof(tcx, ty)
247+
})
248+
} else {
249+
false
250+
}
251+
}

tests/ui/cast.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,3 +501,21 @@ fn issue12721() {
501501
(255 % 999999u64) as u8;
502502
//~^ ERROR: casting `u64` to `u8` may truncate the value
503503
}
504+
505+
pub fn issue_7486() -> u8 {
506+
(2u16 % 256) as u8
507+
}
508+
509+
pub fn issue_9613() {
510+
const CHUNK: usize = 64;
511+
CHUNK as u32;
512+
u64::MIN as u32;
513+
//~^ ERROR: casting `u64` to `u32` may truncate the value
514+
struct Foo(usize, u32);
515+
struct Bar(usize, String);
516+
core::mem::size_of::<Foo>() as u32;
517+
core::mem::size_of::<Bar>() as u32;
518+
//~^ ERROR: casting `usize` to `u32` may truncate
519+
core::mem::size_of::<String>() as u32;
520+
//~^ ERROR: casting `usize` to `u32` may truncate
521+
}

tests/ui/cast.stderr

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -731,5 +731,41 @@ help: ... or use `try_from` and handle the error accordingly
731731
LL | u8::try_from(255 % 999999u64);
732732
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
733733

734-
error: aborting due to 92 previous errors
734+
error: casting `u64` to `u32` may truncate the value
735+
--> tests/ui/cast.rs:512:5
736+
|
737+
LL | u64::MIN as u32;
738+
| ^^^^^^^^^^^^^^^
739+
|
740+
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
741+
help: ... or use `try_from` and handle the error accordingly
742+
|
743+
LL | u32::try_from(u64::MIN);
744+
| ~~~~~~~~~~~~~~~~~~~~~~~
745+
746+
error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers
747+
--> tests/ui/cast.rs:517:5
748+
|
749+
LL | core::mem::size_of::<Bar>() as u32;
750+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
751+
|
752+
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
753+
help: ... or use `try_from` and handle the error accordingly
754+
|
755+
LL | u32::try_from(core::mem::size_of::<Bar>());
756+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
757+
758+
error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers
759+
--> tests/ui/cast.rs:519:5
760+
|
761+
LL | core::mem::size_of::<String>() as u32;
762+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
763+
|
764+
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
765+
help: ... or use `try_from` and handle the error accordingly
766+
|
767+
LL | u32::try_from(core::mem::size_of::<String>());
768+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
769+
770+
error: aborting due to 95 previous errors
735771

0 commit comments

Comments
 (0)