size_of_in_element_count false-positive when dividing byte-size by element size

[MarijnS95]

Lint name: size_of_in_element_count

I tried this code:

fn main() {
    let bytes = [0u8, 1u8, 2u8, 3u8];
    let shorts: &[u16] = unsafe {
        std::slice::from_raw_parts(
            bytes.as_ptr() as *const _,
            bytes.len() / std::mem::size_of::<u16>(),
        )
    };
    dbg!(shorts);
}

I expected to see this happen:

No size_of_in_element_count lint warning/error when dividing by size_of.

Instead, this happened:

    Checking playground v0.0.1 (/playground)
error: found a count of bytes instead of a count of elements of `T`
 --> src/main.rs:6:13
  |
6 |             bytes.len() / std::mem::size_of::<u16>(),
  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[deny(clippy::size_of_in_element_count)]` on by default
  = help: use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count

#6394 recently introduced the size_of_in_element_count to catch cases where byte sizes instead of element sizes are passed into from_raw_parts and related functions. However, size_of is used in the example above to convert away from byte sizes by dividing instead of multiplying. After all the linter error states:

use a count of elements instead of a count of bytes, it already gets multiplied by the size of the type

When no count of bytes nor multiplication is used here at all.

CC @nico-abram

Meta

• cargo +nightly clippy -V: clippy 0.0.212 (257becb 2020-12-27)
• rustc +nightly -Vv:

  rustc 1.51.0-nightly (257becbfe 2020-12-27)
  binary: rustc
  commit-hash: 257becbfe4987d1f7b12af5a8dd5ed96697cd2e8
  commit-date: 2020-12-27
  host: x86_64-unknown-linux-gnu
  release: 1.51.0-nightly
  

[mqy]

I can confirm this problem seen from Apache Arrow.

from_raw_parts(
    self.raw_data() as *const T,
    self.len() / mem::size_of::<T>(),  // <-- clippy error: found a count of bytes instead of a count of elements of `T`
)

Fortunately, the check passed when I extracted the arg len as a variable arg_len:

let arg_len = self.len() / mem::size_of::<T>();
from_raw_parts(
    self.raw_data() as *const T,
    arg_len,
)

[SOF3]

Why was this a deny to start with?

[nico-abram]

I think the intention was to only look for it in the dividend to catch things like len * size_of(...) / chunk_count

The change should be here: https://github.com/nico-abram/rust-clippy/blob/c1a5329475d041dbeb077ecda6ae71f690b4bcc1/clippy_lints/src/size_of_in_element_count.rs#L54-L55

Maybe something like this

        ExprKind::Binary(op, left, right) if BinOpKind::Mul == op.node => {
            get_size_of_ty(cx, left).or_else(|| get_size_of_ty(cx, right))
        },
        ExprKind::Binary(op, left, right) if BinOpKind::Div == op.node => {
            get_size_of_ty(cx, left)
        },

[SOF3]

I'm fine with suppressing this manually, but it should really be a warn.

[MarijnS95]

@nico-abram I wanted to be a bit more conservative and keep track of divisions (inversions) in the RHS. What do you think of this? https://github.com/MarijnS95/rust-clippy/compare/size-in-element-count-divide-by-byte-size?