Skip to content

cast_possible_truncation: Fix some false-positive cases #12962

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

Closed
wants to merge 8 commits into from
Closed
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
60 changes: 55 additions & 5 deletions clippy_lints/src/casts/cast_possible_truncation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize};
use rustc_errors::{Applicability, Diag, SuggestionStyle};
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{BinOpKind, Expr, ExprKind};
use rustc_hir::{BinOpKind, Expr, ExprKind, QPath};
use rustc_lint::LateContext;
use rustc_middle::ty::{self, FloatTy, Ty};
use rustc_middle::ty::{self, FloatTy, Ty, TyCtxt};
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_target::abi::IntegerType;

Expand Down Expand Up @@ -40,8 +41,8 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
get_constant_bits(cx, right).map_or(0, |b| b.saturating_sub(1))
})
},
BinOpKind::Rem => get_constant_bits(cx, right)
.unwrap_or(u64::MAX)
BinOpKind::Rem => constant_int(cx, right)
.map_or(u64::MAX, |c| c.next_power_of_two().trailing_zeros().into())
.min(apply_reductions(cx, nbits, left, signed)),
BinOpKind::BitAnd => get_constant_bits(cx, right)
.unwrap_or(u64::MAX)
Expand Down Expand Up @@ -79,6 +80,35 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
nbits
}
},
ExprKind::Path(QPath::Resolved(
None,
rustc_hir::Path {
res: Res::Def(DefKind::Const | DefKind::AssocConst, def_id),
..
},
))
// NOTE(@Jarcho): A constant from another crate might not have the same value
// for all versions of the crate. This isn't a problem for constants in the
// current crate since a crate can't compile against multiple versions of itself.
if def_id.is_local() => {
// `constant()` already checks if a const item is based on `cfg!`.
get_constant_bits(cx, expr).unwrap_or(nbits)
},
// mem::size_of::<T>();
ExprKind::Call(func, []) => {
if let ExprKind::Path(ref func_qpath) = func.kind
&& let Some(def_id) = cx.qpath_res(func_qpath, func.hir_id).opt_def_id()
&& cx.tcx.is_diagnostic_item(sym::mem_size_of, def_id)
&& let Some(ty) = cx.typeck_results().node_args(func.hir_id).types().next()
&& is_valid_sizeof(cx.tcx, ty)
&& let Ok(layout) = cx.tcx.layout_of(cx.param_env.and(ty))
{
let size: u64 = layout.layout.size().bytes();
(64 - size.leading_zeros()).into()
} else {
nbits
}
},
_ => nbits,
}
}
Expand All @@ -104,7 +134,7 @@ pub(super) fn check(
let (should_lint, suffix) = match (is_isize_or_usize(cast_from), is_isize_or_usize(cast_to)) {
(true, true) | (false, false) => (to_nbits < from_nbits, ""),
(true, false) => (
to_nbits <= 32,
to_nbits < from_nbits && to_nbits <= 32,
if to_nbits == 32 {
" on targets with 64-bit wide pointers"
} else {
Expand Down Expand Up @@ -199,3 +229,23 @@ fn offer_suggestion(
SuggestionStyle::ShowAlways,
);
}

// FIXME(@tesuji): May extend this to a validator functions to include:
// * some ABI-guaranteed STD types,
// * some non-local crate types suggested in [PR-12962][1].
// [1]: https://github.com/rust-lang/rust-clippy/pull/12962#discussion_r1661500351
fn is_valid_sizeof<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
if ty.is_primitive_ty() || ty.is_any_ptr() || ty.is_box() || ty.is_slice() {
return true;
}
if let ty::Adt(def, args) = ty.kind()
&& def.did().is_local()
{
def.all_fields().all(|field| {
let ty = field.ty(tcx, args);
is_valid_sizeof(tcx, ty)
})
} else {
false
}
}
18 changes: 18 additions & 0 deletions tests/ui/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,3 +501,21 @@ fn issue12721() {
(255 % 999999u64) as u8;
//~^ ERROR: casting `u64` to `u8` may truncate the value
}

pub fn issue_7486() -> u8 {
(2u16 % 256) as u8
}

pub fn issue_9613() {
const CHUNK: usize = 64;
CHUNK as u32;
u64::MIN as u32;
//~^ ERROR: casting `u64` to `u32` may truncate the value
struct Foo(usize, u32);
struct Bar(usize, String);
core::mem::size_of::<Foo>() as u32;
core::mem::size_of::<Bar>() as u32;
//~^ ERROR: casting `usize` to `u32` may truncate
core::mem::size_of::<String>() as u32;
//~^ ERROR: casting `usize` to `u32` may truncate
}
38 changes: 37 additions & 1 deletion tests/ui/cast.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -731,5 +731,41 @@ help: ... or use `try_from` and handle the error accordingly
LL | u8::try_from(255 % 999999u64);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 92 previous errors
error: casting `u64` to `u32` may truncate the value
--> tests/ui/cast.rs:512:5
|
LL | u64::MIN as u32;
| ^^^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | u32::try_from(u64::MIN);
| ~~~~~~~~~~~~~~~~~~~~~~~

error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers
--> tests/ui/cast.rs:517:5
|
LL | core::mem::size_of::<Bar>() as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | u32::try_from(core::mem::size_of::<Bar>());
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers
--> tests/ui/cast.rs:519:5
|
LL | core::mem::size_of::<String>() as u32;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ...
help: ... or use `try_from` and handle the error accordingly
|
LL | u32::try_from(core::mem::size_of::<String>());
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 95 previous errors