Repository: pola-rs/polars
File: crates/polars-compute/src/cast/binview_to.rs (Rust)

Code before:
            InvalidOperation:
            "array chunk length * number of items ({} * {}) is too large",
            from.len(),
            array_width
        )
    })?;
    // The size of each array, in bytes:
    let row_size_bytes = element_size.checked_mul(array_width).ok_or_else(|| {
        polars_err!(
            InvalidOperation:
            "array size in bytes ({} * {}) is too large",
            element_size,
            array_width
        )
    })?;

    // Creating references to memory that has not been fully initialized is
    // undefined behavior per
    // https://doc.rust-lang.org/std/primitive.slice.html#method.assume_init
    // so we need everything in `out` to be initialized one way or another.
    //
    // Luckily, initializing as zero is extremely efficient, because once an
    // allocation is big enough, in practice it will become a call to e.g.
    // mmap() which guarantees zero initialization.
    let mut out: Vec<T> = vec![T::zeroed(); primitive_length];
    let out_len_bytes = cast_slice_mut::<_, u8>(out.as_mut()).len();
    assert_eq!(out_len_bytes, row_size_bytes * from.len());
    let mut validity = MutableBitmap::from_len_set(from.len());

    for (index, value) in from.iter().enumerate() {
        if let Some(value) = value
            && value.len() == row_size_bytes
        {
            if cfg!(target_endian = "little") && IS_LITTLE_ENDIAN {
                // Fast path, we can just copy the data with no need to
                // reinterpret.
                let write_index = index * row_size_bytes;
                debug_assert!(value.is_empty() || write_index < out_len_bytes);
                debug_assert!(value.is_empty() || (write_index + value.len() - 1 < out_len_bytes));
                // # Safety
                // - The start index is smaller than `out`'s capacity.
                // - The end index is smaller than `out`'s capacity.
                unsafe {
                    copy_nonoverlapping(
                        value.as_ptr(),
                        (out.as_mut_ptr() as *mut u8).add(write_index),
                        value.len(),
                    );
                }
            } else {
                // Slow path, reinterpret items one by one.

Review comment (suggestion):
Please get the `*mut u8` pointer from the slice returned by `cast_slice`. Store that into a variable `out_u8_ptr` and use that in the loop below instead of `out.as_mut_ptr() as *mut u8`.

The intention is to clearly show anyone reading the code that the `*mut u8` being used originates from the same `&[u8]` slice whose length was asserted.


Code after:
            InvalidOperation:
            "array chunk length * number of items ({} * {}) is too large",
            from.len(),
            array_width
        )
    })?;
    // The size of each array, in bytes:
    let row_size_bytes = element_size.checked_mul(array_width).ok_or_else(|| {
        polars_err!(
            InvalidOperation:
            "array size in bytes ({} * {}) is too large",
            element_size,
            array_width
        )
    })?;

    let mut out: Vec<T> = vec![T::zeroed(); primitive_length];
    let (out_u8_ptr, out_len_bytes) = {
        let out_u8_slice = cast_slice_mut::<_, u8>(out.as_mut());
        (out_u8_slice.as_mut_ptr(), out_u8_slice.len())
    };
    assert_eq!(out_len_bytes, row_size_bytes * from.len());
    let mut validity = MutableBitmap::from_len_set(from.len());

    for (index, value) in from.iter().enumerate() {
        if let Some(value) = value
            && value.len() == row_size_bytes
        {
            if cfg!(target_endian = "little") && IS_LITTLE_ENDIAN {
                // Fast path, we can just copy the data with no need to
                // reinterpret.
                let write_index = index * row_size_bytes;
                debug_assert!(value.is_empty() || write_index < out_len_bytes);
                debug_assert!(value.is_empty() || (write_index + value.len() - 1 < out_len_bytes));
                // # Safety
                // - The start index is smaller than `out`'s capacity.
                // - The end index is smaller than `out`'s capacity.
                unsafe {
                    copy_nonoverlapping(value.as_ptr(), out_u8_ptr.add(write_index), value.len());
                }
            } else {
                // Slow path, reinterpret items one by one.
                for j in 0..array_width {
                    let jth_range = (j * element_size)..((j + 1) * element_size);
                    debug_assert!(value.get(jth_range.clone()).is_some());
                    // # Safety
                    // We made sure the range is smaller than `value` length.
                    let jth_bytes = unsafe { value.get_unchecked(jth_range) };
                    // # Safety
                    // We just made sure that the slice has length `element_size`
                    let byte_array = unsafe { jth_bytes.try_into().unwrap_unchecked() };