From 2080634d336ac73a00aa256993ee035da7226aca Mon Sep 17 00:00:00 2001 From: coekjan Date: Wed, 2 Oct 2024 23:51:52 +0800 Subject: [PATCH] Fix lint `manual_slice_size_calculation` when the slice is ref more than once --- .../src/manual_slice_size_calculation.rs | 14 +++++----- tests/ui/manual_slice_size_calculation.fixed | 4 +++ tests/ui/manual_slice_size_calculation.rs | 4 +++ tests/ui/manual_slice_size_calculation.stderr | 26 ++++++++++++++----- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/manual_slice_size_calculation.rs b/clippy_lints/src/manual_slice_size_calculation.rs index b24a0f4695a6c..a984a7f952052 100644 --- a/clippy_lints/src/manual_slice_size_calculation.rs +++ b/clippy_lints/src/manual_slice_size_calculation.rs @@ -45,10 +45,11 @@ impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation { && !expr.span.from_expansion() // Does not apply inside const because size_of_val is not cost in stable. && !is_in_const_context(cx) - && let Some(receiver) = simplify(cx, left, right) + && let Some((receiver, refs_count)) = simplify(cx, left, right) { let ctxt = expr.span.ctxt(); let mut app = Applicability::MachineApplicable; + let deref = "*".repeat(refs_count - 1); let val_name = snippet_with_context(cx, receiver.span, ctxt, "slice", &mut app).0; let Some(sugg) = std_or_core(cx) else { return }; @@ -58,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation { expr.span, "manual slice size calculation", "try", - format!("{sugg}::mem::size_of_val({val_name})"), + format!("{sugg}::mem::size_of_val({deref}{val_name})"), app, ); } @@ -69,7 +70,7 @@ fn simplify<'tcx>( cx: &LateContext<'tcx>, expr1: &'tcx Expr<'tcx>, expr2: &'tcx Expr<'tcx>, -) -> Option<&'tcx Expr<'tcx>> { +) -> Option<(&'tcx Expr<'tcx>, usize)> { let expr1 = expr_or_init(cx, expr1); let expr2 = expr_or_init(cx, expr2); @@ -80,13 +81,14 @@ fn simplify_half<'tcx>( cx: &LateContext<'tcx>, expr1: &'tcx Expr<'tcx>, expr2: &'tcx Expr<'tcx>, -) -> Option<&'tcx Expr<'tcx>> { +) -> Option<(&'tcx Expr<'tcx>, usize)> { if !expr1.span.from_expansion() // expr1 is `[T1].len()`? && let ExprKind::MethodCall(method_path, receiver, _, _) = expr1.kind && method_path.ident.name == sym::len && let receiver_ty = cx.typeck_results().expr_ty(receiver) - && let ty::Slice(ty1) = receiver_ty.peel_refs().kind() + && let (receiver_ty, refs_count) = clippy_utils::ty::walk_ptrs_ty_depth(receiver_ty) + && let ty::Slice(ty1) = receiver_ty.kind() // expr2 is `size_of::()`? && let ExprKind::Call(func, _) = expr2.kind && let ExprKind::Path(ref func_qpath) = func.kind @@ -96,7 +98,7 @@ fn simplify_half<'tcx>( // T1 == T2? && *ty1 == ty2 { - Some(receiver) + Some((receiver, refs_count)) } else { None } diff --git a/tests/ui/manual_slice_size_calculation.fixed b/tests/ui/manual_slice_size_calculation.fixed index 62b372f4b8d9c..0603b30e34653 100644 --- a/tests/ui/manual_slice_size_calculation.fixed +++ b/tests/ui/manual_slice_size_calculation.fixed @@ -10,11 +10,15 @@ use proc_macros::external; fn main() { let v_i32 = Vec::::new(); let s_i32 = v_i32.as_slice(); + let s_i32_ref = &s_i32; + let s_i32_ref_ref = &s_i32_ref; // True positives: let _ = std::mem::size_of_val(s_i32); // WARNING let _ = std::mem::size_of_val(s_i32); // WARNING let _ = std::mem::size_of_val(s_i32) * 5; // WARNING + let _ = std::mem::size_of_val(*s_i32_ref); // WARNING + let _ = std::mem::size_of_val(**s_i32_ref_ref); // WARNING let len = s_i32.len(); let size = size_of::(); diff --git a/tests/ui/manual_slice_size_calculation.rs b/tests/ui/manual_slice_size_calculation.rs index d59f5fd8b9427..14093e653c09d 100644 --- a/tests/ui/manual_slice_size_calculation.rs +++ b/tests/ui/manual_slice_size_calculation.rs @@ -10,11 +10,15 @@ use proc_macros::external; fn main() { let v_i32 = Vec::::new(); let s_i32 = v_i32.as_slice(); + let s_i32_ref = &s_i32; + let s_i32_ref_ref = &s_i32_ref; // True positives: let _ = s_i32.len() * size_of::(); // WARNING let _ = size_of::() * s_i32.len(); // WARNING let _ = size_of::() * s_i32.len() * 5; // WARNING + let _ = size_of::() * s_i32_ref.len(); // WARNING + let _ = size_of::() * s_i32_ref_ref.len(); // WARNING let len = s_i32.len(); let size = size_of::(); diff --git a/tests/ui/manual_slice_size_calculation.stderr b/tests/ui/manual_slice_size_calculation.stderr index 4bd8a4fdf17b7..0397f3a4969a8 100644 --- a/tests/ui/manual_slice_size_calculation.stderr +++ b/tests/ui/manual_slice_size_calculation.stderr @@ -1,5 +1,5 @@ error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:15:13 + --> tests/ui/manual_slice_size_calculation.rs:17:13 | LL | let _ = s_i32.len() * size_of::(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` @@ -8,40 +8,52 @@ LL | let _ = s_i32.len() * size_of::(); // WARNING = help: to override `-D warnings` add `#[allow(clippy::manual_slice_size_calculation)]` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:16:13 + --> tests/ui/manual_slice_size_calculation.rs:18:13 | LL | let _ = size_of::() * s_i32.len(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:17:13 + --> tests/ui/manual_slice_size_calculation.rs:19:13 | LL | let _ = size_of::() * s_i32.len() * 5; // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` +error: manual slice size calculation + --> tests/ui/manual_slice_size_calculation.rs:20:13 + | +LL | let _ = size_of::() * s_i32_ref.len(); // WARNING + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(*s_i32_ref)` + error: manual slice size calculation --> tests/ui/manual_slice_size_calculation.rs:21:13 | +LL | let _ = size_of::() * s_i32_ref_ref.len(); // WARNING + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(**s_i32_ref_ref)` + +error: manual slice size calculation + --> tests/ui/manual_slice_size_calculation.rs:25:13 + | LL | let _ = len * size_of::(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:22:13 + --> tests/ui/manual_slice_size_calculation.rs:26:13 | LL | let _ = s_i32.len() * size; // WARNING | ^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:23:13 + --> tests/ui/manual_slice_size_calculation.rs:27:13 | LL | let _ = len * size; // WARNING | ^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:25:13 + --> tests/ui/manual_slice_size_calculation.rs:29:13 | LL | let _ = external!(&[1u64][..]).len() * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(external!(&[1u64][..]))` -error: aborting due to 7 previous errors +error: aborting due to 9 previous errors