From 281e4811fbda42e0bc31d946fd6cbe5f9c91f55c Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 21:11:53 +0700 Subject: [PATCH 1/8] Add regression test for 7486 --- tests/ui/cast.rs | 4 ++++ tests/ui/cast.stderr | 14 +++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 146b04ab8131..bd60339a9555 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -501,3 +501,7 @@ fn issue12721() { (255 % 999999u64) as u8; //~^ ERROR: casting `u64` to `u8` may truncate the value } + +pub fn issue_7486() -> u8 { + (2u16 % 256) as u8 +} diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 7824bdfac25e..36ac2cf3fdd3 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,5 +731,17 @@ 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 `u16` to `u8` may truncate the value + --> tests/ui/cast.rs:503:5 + | +LL | (2u16 % 256) as u8 + | ^^^^^^^^^^^^^^^^^^ + | + = 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 | u8::try_from(2u16 % 256) + | + +error: aborting due to 93 previous errors From c421cb260f7bdd12b94faaa8b87586a6790d9d30 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 21:14:12 +0700 Subject: [PATCH 2/8] Fix off-by-one on % operator of cast_possible_truncation --- clippy_lints/src/casts/cast_possible_truncation.rs | 4 ++-- tests/ui/cast.stderr | 14 +------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 7c5acd1a678d..31e21618fbf4 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -40,8 +40,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) diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 36ac2cf3fdd3..7824bdfac25e 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,17 +731,5 @@ help: ... or use `try_from` and handle the error accordingly LL | u8::try_from(255 % 999999u64); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: casting `u16` to `u8` may truncate the value - --> tests/ui/cast.rs:503:5 - | -LL | (2u16 % 256) as u8 - | ^^^^^^^^^^^^^^^^^^ - | - = 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 | u8::try_from(2u16 % 256) - | - -error: aborting due to 93 previous errors +error: aborting due to 92 previous errors From 89a4cdfb7b978c9597f529ab45d82b1b20424996 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 21:23:57 +0700 Subject: [PATCH 3/8] add regression test 9613 --- tests/ui/cast.rs | 5 +++++ tests/ui/cast.stderr | 14 +++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index bd60339a9555..34867ff68503 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -505,3 +505,8 @@ fn issue12721() { pub fn issue_7486() -> u8 { (2u16 % 256) as u8 } + +pub fn issue_9613() { + const CHUNK: usize = 64; + CHUNK as u32; +} diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 7824bdfac25e..41daa8810b4b 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,5 +731,17 @@ 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 `usize` to `u32` may truncate the value on targets with 64-bit wide pointers + --> tests/ui/cast.rs:508:5 + | +LL | CHUNK 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(CHUNK); + | ~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 93 previous errors From 391e9b3fe8656750fa94ee93bf59008165c5a1c3 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 21:51:42 +0700 Subject: [PATCH 4/8] cast_possible_truncation: don't lint on valid constants --- clippy_lints/src/casts/cast_possible_truncation.rs | 3 ++- tests/ui/cast.stderr | 14 +------------- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 31e21618fbf4..2022179bfbdb 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -79,6 +79,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b nbits } }, + ExprKind::Path(ref _qpath) => get_constant_bits(cx, expr).unwrap_or(nbits), _ => nbits, } } @@ -104,7 +105,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 { diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 41daa8810b4b..7824bdfac25e 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,17 +731,5 @@ help: ... or use `try_from` and handle the error accordingly LL | u8::try_from(255 % 999999u64); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast.rs:508:5 - | -LL | CHUNK 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(CHUNK); - | ~~~~~~~~~~~~~~~~~~~~ - -error: aborting due to 93 previous errors +error: aborting due to 92 previous errors From b7461142258dbbb005525ec01297608df671a6dd Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 22:03:50 +0700 Subject: [PATCH 5/8] add test for size_of::() in 9613 --- tests/ui/cast.rs | 1 + tests/ui/cast.stderr | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 34867ff68503..9114fcac503f 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -509,4 +509,5 @@ pub fn issue_7486() -> u8 { pub fn issue_9613() { const CHUNK: usize = 64; CHUNK as u32; + core::mem::size_of::() as u32; } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 7824bdfac25e..6a6c25985f15 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,5 +731,17 @@ 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 `usize` to `u32` may truncate the value on targets with 64-bit wide pointers + --> tests/ui/cast.rs:509:5 + | +LL | core::mem::size_of::() 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::()); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 93 previous errors From d05e9d798ae92c650a6e950d70a9fc791f9cef40 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Wed, 19 Jun 2024 22:29:43 +0700 Subject: [PATCH 6/8] cast_possible_truncation: do not lint on size_of --- .../src/casts/cast_possible_truncation.rs | 17 ++++++++++++++++- tests/ui/cast.stderr | 14 +------------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 2022179bfbdb..7eb04aa2ceb9 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -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_span::symbol::sym; use rustc_span::Span; use rustc_target::abi::IntegerType; @@ -80,6 +81,20 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b } }, ExprKind::Path(ref _qpath) => get_constant_bits(cx, expr).unwrap_or(nbits), + // mem::size_of::(); + 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() + && 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, } } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 6a6c25985f15..7824bdfac25e 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,17 +731,5 @@ help: ... or use `try_from` and handle the error accordingly LL | u8::try_from(255 % 999999u64); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -error: casting `usize` to `u32` may truncate the value on targets with 64-bit wide pointers - --> tests/ui/cast.rs:509:5 - | -LL | core::mem::size_of::() 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::()); - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -error: aborting due to 93 previous errors +error: aborting due to 92 previous errors From e998bb816814ef858570fb78b523f3d506807814 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Tue, 2 Jul 2024 08:39:37 +0700 Subject: [PATCH 7/8] limit eval const items to the current local crate --- .../src/casts/cast_possible_truncation.rs | 15 ++++++++++++++- tests/ui/cast.rs | 2 ++ tests/ui/cast.stderr | 14 +++++++++++++- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 7eb04aa2ceb9..6db5249b266f 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -80,7 +80,20 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b nbits } }, - ExprKind::Path(ref _qpath) => get_constant_bits(cx, expr).unwrap_or(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::(); ExprKind::Call(func, []) => { if let ExprKind::Path(ref func_qpath) = func.kind diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 9114fcac503f..07132cdd689f 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -509,5 +509,7 @@ pub fn issue_7486() -> 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 core::mem::size_of::() as u32; } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 7824bdfac25e..1880caa3d137 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -731,5 +731,17 @@ 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:510: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: aborting due to 93 previous errors From 3770c8f12de37d1460e346a0239d2651330d1114 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Mon, 15 Jul 2024 16:58:43 +0700 Subject: [PATCH 8/8] Limit local ADT types to size_of Require all fields to be local --- .../src/casts/cast_possible_truncation.rs | 23 ++++++++++++++- tests/ui/cast.rs | 6 ++++ tests/ui/cast.stderr | 28 +++++++++++++++++-- 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index 6db5249b266f..b4306bbe6be4 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -8,7 +8,7 @@ use rustc_errors::{Applicability, Diag, SuggestionStyle}; use rustc_hir::def::{DefKind, Res}; 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; @@ -100,6 +100,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b && 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(); @@ -228,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 + } +} diff --git a/tests/ui/cast.rs b/tests/ui/cast.rs index 07132cdd689f..ed67c842569c 100644 --- a/tests/ui/cast.rs +++ b/tests/ui/cast.rs @@ -511,5 +511,11 @@ pub fn issue_9613() { 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::() as u32; + core::mem::size_of::() as u32; + //~^ ERROR: casting `usize` to `u32` may truncate core::mem::size_of::() as u32; + //~^ ERROR: casting `usize` to `u32` may truncate } diff --git a/tests/ui/cast.stderr b/tests/ui/cast.stderr index 1880caa3d137..e8cd5e2601ad 100644 --- a/tests/ui/cast.stderr +++ b/tests/ui/cast.stderr @@ -732,7 +732,7 @@ LL | u8::try_from(255 % 999999u64); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: casting `u64` to `u32` may truncate the value - --> tests/ui/cast.rs:510:5 + --> tests/ui/cast.rs:512:5 | LL | u64::MIN as u32; | ^^^^^^^^^^^^^^^ @@ -743,5 +743,29 @@ help: ... or use `try_from` and handle the error accordingly LL | u32::try_from(u64::MIN); | ~~~~~~~~~~~~~~~~~~~~~~~ -error: aborting due to 93 previous errors +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::() 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::()); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +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::() 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::()); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 95 previous errors