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

Code before:
            element_size,
            array_width
        )
    })?;
    let mut out: Vec<T> = Vec::with_capacity(primitive_length);
    let mut validity = MutableBitmap::from_len_set(from.len());

    for (index, value) in from.iter().enumerate() {
        if let Some(value) = value
            && value.len() == array_bytes_size
        {
            if cfg!(target_endian = "little") && IS_LITTLE_ENDIAN {
                // Fast path, we can just copy the data with no need to
                // reinterpret.
                let write_index = array_width * index;
                debug_assert!(write_index < primitive_length);
                debug_assert!((write_index + (array_width - 1)) < primitive_length);
                // # Safety
                // - The target index is smaller than the vector's pre-allocated
                //   capacity.
                // - We made sure `value` has byte length
                //   `array_width * element_size`.
                unsafe {
                    copy_nonoverlapping(
                        value.as_ptr(),
                        out.as_mut_ptr().add(write_index) as *mut u8,
                        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() };
                    let jth_value = if IS_LITTLE_ENDIAN {
                        <T as FromBytes>::from_le_bytes(byte_array)
                    } else {
                        <T as FromBytes>::from_be_bytes(byte_array)
                    };

                    let write_index = array_width * index + j;
                    debug_assert!(write_index < primitive_length);
                    // # Safety
                    // - The target index is smaller than the vector's pre-allocated capacity.
                    unsafe {

Review comment (suggestion):
Instead of performing pointer arithmetic on `*mut T` and then casting to `*mut u8`, please perform these steps upfront:

* In the code before the loop, get the `&[T]` from the `Vec<T>`, then convert it to a `&[u8]` using `bytemuck::cast_slice`.
  * Then add an assertion that `&[u8].len() == array_bytes_size * from.len()`. ~~You may need to move the `set_len` up from below the function to achieve this.~~
  * Then, get the `*mut u8` representing the beginning of this slice
* Then, in this branch, perform pointer arithmetic directly based off the `*mut u8` - i.e. `u8_base.add(array_bytes_size * index)`

That makes it much clearer and easier to verify that the writes are all within bounds.



Code after:
            "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() };
                    let jth_value = if IS_LITTLE_ENDIAN {
                        <T as FromBytes>::from_le_bytes(byte_array)
                    } else {
                        <T as FromBytes>::from_be_bytes(byte_array)
                    };

                    let write_index = array_width * index + j;
                    debug_assert!(write_index < out.len());
                    // # Safety
                    // - The target index is smaller than the vector's pre-allocated capacity.