Auto merge of #6578 - MarijnS95:size-in-element-count-divide-by-byte-size, r=flip1995

size_of_in_element_count: Disable lint on division by byte-size

Fixes #6511

It is fairly common to divide some length in bytes by the byte-size of a single element before creating a `from_raw_parts` slice or similar operation. This lint would erroneously disallow such expressions.

Just in case, instead of simply disabling this lint in the RHS of a division, keep track of the inversion and enable it again on recursive division.

---

changelog: Do not trigger size_of_in_element_count when dividing by element size

5 files changed, 103 insertions, 66 deletions

diff --git a/clippy_lints/src/size_of_in_element_count.rs b/clippy_lints/src/size_of_in_element_count.rs
index ea7a76146f5..87e386baadc 100644
--- a/clippy_lints/src/size_of_in_element_count.rs
+++ b/clippy_lints/src/size_of_in_element_count.rs
@@ -35,10 +35,11 @@ declare_clippy_lint! {
 
 declare_lint_pass!(SizeOfInElementCount => [SIZE_OF_IN_ELEMENT_COUNT]);
 
-fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tcx>> {
+fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, inverted: bool) -> Option<Ty<'tcx>> {
     match expr.kind {
         ExprKind::Call(count_func, _func_args) => {
             if_chain! {
+                if !inverted;
                 if let ExprKind::Path(ref count_func_qpath) = count_func.kind;
                 if let Some(def_id) = cx.qpath_res(count_func_qpath, count_func.hir_id).opt_def_id();
                 if match_def_path(cx, def_id, &paths::MEM_SIZE_OF)
@@ -50,10 +51,13 @@ fn get_size_of_ty(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<Ty<'tc
                 }
             }
         },
-        ExprKind::Binary(op, left, right) if BinOpKind::Mul == op.node || BinOpKind::Div == op.node => {
-            get_size_of_ty(cx, left).or_else(|| get_size_of_ty(cx, right))
+        ExprKind::Binary(op, left, right) if BinOpKind::Mul == op.node => {
+            get_size_of_ty(cx, left, inverted).or_else(|| get_size_of_ty(cx, right, inverted))
         },
-        ExprKind::Cast(expr, _) => get_size_of_ty(cx, expr),
+        ExprKind::Binary(op, left, right) if BinOpKind::Div == op.node => {
+            get_size_of_ty(cx, left, inverted).or_else(|| get_size_of_ty(cx, right, !inverted))
+        },
+        ExprKind::Cast(expr, _) => get_size_of_ty(cx, expr, inverted),
         _ => None,
     }
 }
@@ -128,7 +132,7 @@ impl<'tcx> LateLintPass<'tcx> for SizeOfInElementCount {
 
             // Find a size_of call in the count parameter expression and
             // check that it's the same type
-            if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count_expr);
+            if let Some(ty_used_for_size_of) = get_size_of_ty(cx, count_expr, false);
             if TyS::same_type(pointee_ty, ty_used_for_size_of);
             then {
                 span_lint_and_help(
diff --git a/tests/ui/size_of_in_element_count/expressions.rs b/tests/ui/size_of_in_element_count/expressions.rs
new file mode 100644
index 00000000000..2594e8fa6ad
--- /dev/null
+++ b/tests/ui/size_of_in_element_count/expressions.rs
@@ -0,0 +1,37 @@
+#![warn(clippy::size_of_in_element_count)]
+#![allow(clippy::ptr_offset_with_cast)]
+
+use std::mem::{size_of, size_of_val};
+use std::ptr::{copy, copy_nonoverlapping, write_bytes};
+
+fn main() {
+    const SIZE: usize = 128;
+    const HALF_SIZE: usize = SIZE / 2;
+    const DOUBLE_SIZE: usize = SIZE * 2;
+    let mut x = [2u8; SIZE];
+    let mut y = [2u8; SIZE];
+
+    // Count expression involving multiplication of size_of (Should trigger the lint)
+    unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
+
+    // Count expression involving nested multiplications of size_of (Should trigger the lint)
+    unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
+
+    // Count expression involving divisions of size_of (Should trigger the lint)
+    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
+
+    // Count expression involving divisions by size_of (Should not trigger the lint)
+    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / size_of::<u8>()) };
+
+    // Count expression involving divisions by multiple size_of (Should not trigger the lint)
+    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 * size_of::<u8>())) };
+
+    // Count expression involving recursive divisions by size_of (Should trigger the lint)
+    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 / size_of::<u8>())) };
+
+    // No size_of calls (Should not trigger the lint)
+    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), SIZE) };
+
+    // Different types for pointee and size_of (Should not trigger the lint)
+    unsafe { y.as_mut_ptr().write_bytes(0u8, size_of::<u16>() / 2 * SIZE) };
+}
diff --git a/tests/ui/size_of_in_element_count/expressions.stderr b/tests/ui/size_of_in_element_count/expressions.stderr
new file mode 100644
index 00000000000..0f0dff57f51
--- /dev/null
+++ b/tests/ui/size_of_in_element_count/expressions.stderr
@@ -0,0 +1,35 @@
+error: found a count of bytes instead of a count of elements of `T`
+  --> $DIR/expressions.rs:15:62
+   |
+LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
+   |                                                              ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::size-of-in-element-count` implied by `-D warnings`
+   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
+
+error: found a count of bytes instead of a count of elements of `T`
+  --> $DIR/expressions.rs:18:62
+   |
+LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
+   |                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
+
+error: found a count of bytes instead of a count of elements of `T`
+  --> $DIR/expressions.rs:21:47
+   |
+LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
+   |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
+
+error: found a count of bytes instead of a count of elements of `T`
+  --> $DIR/expressions.rs:30:47
+   |
+LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE / (2 / size_of::<u8>())) };
+   |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
+
+error: aborting due to 4 previous errors
+
diff --git a/tests/ui/size_of_in_element_count.rs b/tests/ui/size_of_in_element_count/functions.rs
index b13e390705a..09d08ac37dc 100644
--- a/tests/ui/size_of_in_element_count.rs
+++ b/tests/ui/size_of_in_element_count/functions.rs
@@ -43,19 +43,4 @@ fn main() {
     y.as_mut_ptr().wrapping_add(size_of::<u8>());
     unsafe { y.as_ptr().offset(size_of::<u8>() as isize) };
     y.as_mut_ptr().wrapping_offset(size_of::<u8>() as isize);
-
-    // Count expression involving multiplication of size_of (Should trigger the lint)
-    unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
-
-    // Count expression involving nested multiplications of size_of (Should trigger the lint)
-    unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
-
-    // Count expression involving divisions of size_of (Should trigger the lint)
-    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
-
-    // No size_of calls (Should not trigger the lint)
-    unsafe { copy(x.as_ptr(), y.as_mut_ptr(), SIZE) };
-
-    // Different types for pointee and size_of (Should not trigger the lint)
-    unsafe { y.as_mut_ptr().write_bytes(0u8, size_of::<u16>() / 2 * SIZE) };
 }
diff --git a/tests/ui/size_of_in_element_count.stderr b/tests/ui/size_of_in_element_count/functions.stderr
index 8cf3612abda..c1e824167b7 100644
--- a/tests/ui/size_of_in_element_count.stderr
+++ b/tests/ui/size_of_in_element_count/functions.stderr
@@ -1,5 +1,5 @@
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:18:68
+  --> $DIR/functions.rs:18:68
    |
 LL |     unsafe { copy_nonoverlapping::<u8>(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
    |                                                                    ^^^^^^^^^^^^^^^
@@ -8,7 +8,7 @@ LL |     unsafe { copy_nonoverlapping::<u8>(x.as_ptr(), y.as_mut_ptr(), size_of:
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:19:62
+  --> $DIR/functions.rs:19:62
    |
 LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
    |                                                              ^^^^^^^^^^^^^^^^^^
@@ -16,7 +16,7 @@ LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:21:49
+  --> $DIR/functions.rs:21:49
    |
 LL |     unsafe { x.as_ptr().copy_to(y.as_mut_ptr(), size_of::<u8>()) };
    |                                                 ^^^^^^^^^^^^^^^
@@ -24,7 +24,7 @@ LL |     unsafe { x.as_ptr().copy_to(y.as_mut_ptr(), size_of::<u8>()) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:22:64
+  --> $DIR/functions.rs:22:64
    |
 LL |     unsafe { x.as_ptr().copy_to_nonoverlapping(y.as_mut_ptr(), size_of::<u8>()) };
    |                                                                ^^^^^^^^^^^^^^^
@@ -32,7 +32,7 @@ LL |     unsafe { x.as_ptr().copy_to_nonoverlapping(y.as_mut_ptr(), size_of::<u8
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:23:51
+  --> $DIR/functions.rs:23:51
    |
 LL |     unsafe { y.as_mut_ptr().copy_from(x.as_ptr(), size_of::<u8>()) };
    |                                                   ^^^^^^^^^^^^^^^
@@ -40,7 +40,7 @@ LL |     unsafe { y.as_mut_ptr().copy_from(x.as_ptr(), size_of::<u8>()) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:24:66
+  --> $DIR/functions.rs:24:66
    |
 LL |     unsafe { y.as_mut_ptr().copy_from_nonoverlapping(x.as_ptr(), size_of::<u8>()) };
    |                                                                  ^^^^^^^^^^^^^^^
@@ -48,7 +48,7 @@ LL |     unsafe { y.as_mut_ptr().copy_from_nonoverlapping(x.as_ptr(), size_of::<
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:26:47
+  --> $DIR/functions.rs:26:47
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
    |                                               ^^^^^^^^^^^^^^^
@@ -56,7 +56,7 @@ LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>()) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:27:47
+  --> $DIR/functions.rs:27:47
    |
 LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
    |                                               ^^^^^^^^^^^^^^^^^^
@@ -64,7 +64,7 @@ LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), size_of_val(&x[0])) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:29:46
+  --> $DIR/functions.rs:29:46
    |
 LL |     unsafe { y.as_mut_ptr().write_bytes(0u8, size_of::<u8>() * SIZE) };
    |                                              ^^^^^^^^^^^^^^^^^^^^^^
@@ -72,7 +72,7 @@ LL |     unsafe { y.as_mut_ptr().write_bytes(0u8, size_of::<u8>() * SIZE) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:30:47
+  --> $DIR/functions.rs:30:47
    |
 LL |     unsafe { write_bytes(y.as_mut_ptr(), 0u8, size_of::<u8>() * SIZE) };
    |                                               ^^^^^^^^^^^^^^^^^^^^^^
@@ -80,7 +80,7 @@ LL |     unsafe { write_bytes(y.as_mut_ptr(), 0u8, size_of::<u8>() * SIZE) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:32:66
+  --> $DIR/functions.rs:32:66
    |
 LL |     unsafe { swap_nonoverlapping(y.as_mut_ptr(), x.as_mut_ptr(), size_of::<u8>() * SIZE) };
    |                                                                  ^^^^^^^^^^^^^^^^^^^^^^
@@ -88,7 +88,7 @@ LL |     unsafe { swap_nonoverlapping(y.as_mut_ptr(), x.as_mut_ptr(), size_of::<
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:34:46
+  --> $DIR/functions.rs:34:46
    |
 LL |     slice_from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE);
    |                                              ^^^^^^^^^^^^^^^^^^^^^^
@@ -96,7 +96,7 @@ LL |     slice_from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE);
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:35:38
+  --> $DIR/functions.rs:35:38
    |
 LL |     slice_from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE);
    |                                      ^^^^^^^^^^^^^^^^^^^^^^
@@ -104,7 +104,7 @@ LL |     slice_from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE);
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:37:49
+  --> $DIR/functions.rs:37:49
    |
 LL |     unsafe { from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE) };
    |                                                 ^^^^^^^^^^^^^^^^^^^^^^
@@ -112,7 +112,7 @@ LL |     unsafe { from_raw_parts_mut(y.as_mut_ptr(), size_of::<u8>() * SIZE) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:38:41
+  --> $DIR/functions.rs:38:41
    |
 LL |     unsafe { from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE) };
    |                                         ^^^^^^^^^^^^^^^^^^^^^^
@@ -120,7 +120,7 @@ LL |     unsafe { from_raw_parts(y.as_ptr(), size_of::<u8>() * SIZE) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:40:33
+  --> $DIR/functions.rs:40:33
    |
 LL |     unsafe { y.as_mut_ptr().sub(size_of::<u8>()) };
    |                                 ^^^^^^^^^^^^^^^
@@ -128,7 +128,7 @@ LL |     unsafe { y.as_mut_ptr().sub(size_of::<u8>()) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:41:29
+  --> $DIR/functions.rs:41:29
    |
 LL |     y.as_ptr().wrapping_sub(size_of::<u8>());
    |                             ^^^^^^^^^^^^^^^
@@ -136,7 +136,7 @@ LL |     y.as_ptr().wrapping_sub(size_of::<u8>());
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:42:29
+  --> $DIR/functions.rs:42:29
    |
 LL |     unsafe { y.as_ptr().add(size_of::<u8>()) };
    |                             ^^^^^^^^^^^^^^^
@@ -144,7 +144,7 @@ LL |     unsafe { y.as_ptr().add(size_of::<u8>()) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:43:33
+  --> $DIR/functions.rs:43:33
    |
 LL |     y.as_mut_ptr().wrapping_add(size_of::<u8>());
    |                                 ^^^^^^^^^^^^^^^
@@ -152,7 +152,7 @@ LL |     y.as_mut_ptr().wrapping_add(size_of::<u8>());
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:44:32
+  --> $DIR/functions.rs:44:32
    |
 LL |     unsafe { y.as_ptr().offset(size_of::<u8>() as isize) };
    |                                ^^^^^^^^^^^^^^^^^^^^^^^^
@@ -160,36 +160,12 @@ LL |     unsafe { y.as_ptr().offset(size_of::<u8>() as isize) };
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
 error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:45:36
+  --> $DIR/functions.rs:45:36
    |
 LL |     y.as_mut_ptr().wrapping_offset(size_of::<u8>() as isize);
    |                                    ^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
 
-error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:48:62
-   |
-LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), size_of::<u8>() * SIZE) };
-   |                                                              ^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
-
-error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:51:62
-   |
-LL |     unsafe { copy_nonoverlapping(x.as_ptr(), y.as_mut_ptr(), HALF_SIZE * size_of_val(&x[0]) * 2) };
-   |                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
-
-error: found a count of bytes instead of a count of elements of `T`
-  --> $DIR/size_of_in_element_count.rs:54:47
-   |
-LL |     unsafe { copy(x.as_ptr(), y.as_mut_ptr(), DOUBLE_SIZE * size_of::<u8>() / 2) };
-   |                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   |
-   = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
-
-error: aborting due to 24 previous errors
+error: aborting due to 21 previous errors