Re: [PATCH 07/10] rust: Add arrayvec

Andrew Ballance
2025-03-26T21:06:36Z

On Wed, Mar 26, 2025 at 12:13 PM Remo Senekowitsch Wrote:
> +pub struct ArrayVec<const N: usize, T> {
> + array: [core::mem::MaybeUninit<T>; N],
> + len: usize,
> +}

I would reverse the order of the generic arguments so T is first so
that it matches the declaration of an array: [T; N]

> + /// Adds a new element to the end of the vector.
> + ///
> + /// # Panics
> + ///
> + /// Panics if the vector is already full.
> + pub fn push(&mut self, elem: T) {
> + if self.len == N {
> + panic!("OOM")
> + }
> + self.array[self.len] = MaybeUninit::new(elem);
> + self.len += 1;
> + }

This function should not panic. I would rewrite it so that its signature is
pub fn push(&mut self, elem: T) -> Result<(), T>

> +impl<const N: usize, T> AsRef<[T]> for ArrayVec<N, T> {
> + fn as_ref(&self) -> &[T] {
> + // SAFETY: As per the type invariant, all elements at index < self.len
> + // are initialized.
> + unsafe { core::mem::transmute(&self.array[..self.len]) }
> + }
> +}
> +
> +impl<const N: usize, T> AsMut<[T]> for ArrayVec<N, T> {
> + fn as_mut(&mut self) -> &mut [T] {
> + // SAFETY: As per the type invariant, all elements at index < self.len
> + // are initialized.
> + unsafe { core::mem::transmute(&mut self.array[..self.len]) }
> + }
> +}

I would replace both uses of transmute here with slice::from_raw_parts
like you did in drop.

also it's not needed but you could also impl Deref and DerefMut for ArrayVec
because you have already impl'ed AsRef and AsMut so it should be simple

> +impl<const N: usize, T> Drop for ArrayVec<N, T> {
> + fn drop(&mut self) {
> + unsafe {

this is missing a safety comment

> + let slice: &mut [T] =
> + core::slice::from_raw_parts_mut(self.array.as_mut_ptr().cast(), self.len);
> + core::ptr::drop_in_place(slice);
> + }
> + }
> +}

Andrew