Auto merge of #10601 - schubart:manual_slice_size_calculation, r=llogiq

Add [`manual_slice_size_calculation`]

Fixes: #10518

---

changelog: new lint [`manual_slice_size_calculation`]

Author: bors

CHANGELOG.md

@@ -4674,6 +4674,7 @@ Released 2018-09-13
 [`manual_rem_euclid`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid
 [`manual_retain`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
+[`manual_slice_size_calculation`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_slice_size_calculation
 [`manual_split_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_split_once
 [`manual_str_repeat`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_str_repeat
 [`manual_string_new`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_string_new

clippy_lints/src/declared_lints.rs

@@ -269,6 +269,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
     crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
     crate::manual_retain::MANUAL_RETAIN_INFO,
+    crate::manual_slice_size_calculation::MANUAL_SLICE_SIZE_CALCULATION_INFO,
     crate::manual_string_new::MANUAL_STRING_NEW_INFO,
     crate::manual_strip::MANUAL_STRIP_INFO,
     crate::map_unit_fn::OPTION_MAP_UNIT_FN_INFO,

clippy_lints/src/lib.rs

@@ -185,6 +185,7 @@ mod manual_main_separator_str;
 mod manual_non_exhaustive;
 mod manual_rem_euclid;
 mod manual_retain;
+mod manual_slice_size_calculation;
 mod manual_string_new;
 mod manual_strip;
 mod map_unit_fn;
@@ -956,6 +957,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     });
     store.register_late_pass(|_| Box::new(lines_filter_map_ok::LinesFilterMapOk));
     store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule));
+    store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/manual_slice_size_calculation.rs

@@ -0,0 +1,93 @@
+use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::{expr_or_init, in_constant};
+use rustc_hir::{BinOpKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::symbol::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// When `a` is `&[T]`, detect `a.len() * size_of::<T>()` and suggest `size_of_val(a)`
+    /// instead.
+    ///
+    /// ### Why is this better?
+    /// * Shorter to write
+    /// * Removes the need for the human and the compiler to worry about overflow in the
+    ///   multiplication
+    /// * Potentially faster at runtime as rust emits special no-wrapping flags when it
+    ///   calculates the byte length
+    /// * Less turbofishing
+    ///
+    /// ### Example
+    /// ```rust
+    /// # let data : &[i32] = &[1, 2, 3];
+    /// let newlen = data.len() * std::mem::size_of::<i32>();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # let data : &[i32] = &[1, 2, 3];
+    /// let newlen = std::mem::size_of_val(data);
+    /// ```
+    #[clippy::version = "1.70.0"]
+    pub MANUAL_SLICE_SIZE_CALCULATION,
+    complexity,
+    "manual slice size calculation"
+}
+declare_lint_pass!(ManualSliceSizeCalculation => [MANUAL_SLICE_SIZE_CALCULATION]);
+
+impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        // Does not apply inside const because size_of_value is not cost in stable.
+        if !in_constant(cx, expr.hir_id)
+            && let ExprKind::Binary(ref op, left, right) = expr.kind
+            && BinOpKind::Mul == op.node
+            && let Some(_receiver) = simplify(cx, left, right)
+        {
+            span_lint_and_help(
+                cx,
+                MANUAL_SLICE_SIZE_CALCULATION,
+                expr.span,
+                "manual slice size calculation",
+                None,
+                "consider using std::mem::size_of_value instead");
+        }
+    }
+}
+
+fn simplify<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr1: &'tcx Expr<'tcx>,
+    expr2: &'tcx Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    let expr1 = expr_or_init(cx, expr1);
+    let expr2 = expr_or_init(cx, expr2);
+
+    simplify_half(cx, expr1, expr2).or_else(|| simplify_half(cx, expr2, expr1))
+}
+
+fn simplify_half<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr1: &'tcx Expr<'tcx>,
+    expr2: &'tcx Expr<'tcx>,
+) -> Option<&'tcx Expr<'tcx>> {
+    if
+        // 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()
+        // expr2 is `size_of::<T2>()`?
+        && let ExprKind::Call(func, _) = expr2.kind
+        && 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(ty2) = cx.typeck_results().node_substs(func.hir_id).types().next()
+        // T1 == T2?
+        && *ty1 == ty2
+    {
+        Some(receiver)
+    } else {
+        None
+    }
+}

tests/ui/manual_slice_size_calculation.rs

@@ -0,0 +1,36 @@
+#![allow(unused)]
+#![warn(clippy::manual_slice_size_calculation)]
+
+use core::mem::{align_of, size_of};
+
+fn main() {
+    let v_i32 = Vec::<i32>::new();
+    let s_i32 = v_i32.as_slice();
+
+    // True positives:
+    let _ = s_i32.len() * size_of::<i32>(); // WARNING
+    let _ = size_of::<i32>() * s_i32.len(); // WARNING
+    let _ = size_of::<i32>() * s_i32.len() * 5; // WARNING
+
+    let len = s_i32.len();
+    let size = size_of::<i32>();
+    let _ = len * size_of::<i32>(); // WARNING
+    let _ = s_i32.len() * size; // WARNING
+    let _ = len * size; // WARNING
+
+    // True negatives:
+    let _ = size_of::<i32>() + s_i32.len(); // Ok, not a multiplication
+    let _ = size_of::<i32>() * s_i32.partition_point(|_| true); // Ok, not len()
+    let _ = size_of::<i32>() * v_i32.len(); // Ok, not a slice
+    let _ = align_of::<i32>() * s_i32.len(); // Ok, not size_of()
+    let _ = size_of::<u32>() * s_i32.len(); // Ok, different types
+
+    // False negatives:
+    let _ = 5 * size_of::<i32>() * s_i32.len(); // Ok (MISSED OPPORTUNITY)
+    let _ = size_of::<i32>() * 5 * s_i32.len(); // Ok (MISSED OPPORTUNITY)
+}
+
+const fn _const(s_i32: &[i32]) {
+    // True negative:
+    let _ = s_i32.len() * size_of::<i32>(); // Ok, can't use size_of_val in const
+}

tests/ui/manual_slice_size_calculation.stderr

@@ -0,0 +1,51 @@
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:11:13
+   |
+LL |     let _ = s_i32.len() * size_of::<i32>(); // WARNING
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+   = note: `-D clippy::manual-slice-size-calculation` implied by `-D warnings`
+
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:12:13
+   |
+LL |     let _ = size_of::<i32>() * s_i32.len(); // WARNING
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:13:13
+   |
+LL |     let _ = size_of::<i32>() * s_i32.len() * 5; // WARNING
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:17:13
+   |
+LL |     let _ = len * size_of::<i32>(); // WARNING
+   |             ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:18:13
+   |
+LL |     let _ = s_i32.len() * size; // WARNING
+   |             ^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+
+error: manual slice size calculation
+  --> $DIR/manual_slice_size_calculation.rs:19:13
+   |
+LL |     let _ = len * size; // WARNING
+   |             ^^^^^^^^^^
+   |
+   = help: consider using std::mem::size_of_value instead
+
+error: aborting due to 6 previous errors
+