Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions soavec/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,55 @@ impl<T: SoAble> SoAVec<T> {
unsafe { T::TupleRepr::drop_in_place(T::TupleRepr::get_pointers(ptr, 0, cap), len) };
}
}

/// Removes an element from the vector and returns it.
///
/// The removed element is replaced by the last element of the vector.
///
/// This does not preserve ordering of the remaining elements, but is *O*(n_fields).
/// If you need to preserve the element order, use [`remove`] instead.
///
/// [`remove`]: SoAVec::remove
///
/// # Examples
///
/// ```
/// use soavec::soavec;
///
/// let mut v = soavec![("foo", "foo"), ("bar", "bar"), ("baz", "baz"), ("qux", "qux")].unwrap();
///
/// assert_eq!(v.swap_remove(1).unwrap(), ("bar", "bar"));
/// assert_eq!(v, soavec![("foo", "foo"), ("qux", "qux"), ("baz", "baz")].unwrap());
///
/// assert_eq!(v.swap_remove(0).unwrap(), ("foo", "foo"));
/// assert_eq!(v, soavec![("baz", "baz"), ("qux", "qux")].unwrap());
/// ```
pub fn swap_remove(&mut self, index: u32) -> Result<T, InsertError> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I don't think InsertError is a good choice here since this can never allocate and thus doesn't need those errors. Let's just use IndexOutOfBoundsError directly.

#[cold]
fn assert_index() -> IndexOutOfBoundsError {
IndexOutOfBoundsError
}

let len = self.len();
if index > len {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: This should be index >= len I believe; eg. for a length 0 Vec/SoAVec it should be invalid to remove any index, including 0.

return Err(assert_index().into());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue: I was going to suggest using eg. gdb or equivalent to check out the assembly generated here but I went and checked it out using Compiler Explorer: turns out we don't need the IndexOutOfBoundsError check here at all; the compiler does understand that returning Err is likely a cold branch and optimises that to the end of the function (see the LBB0_1 label: that's the inside of the if branch).

So, in that case it's not necessary to add the #[cold] function and it's entirely possible that it would be harmful: the compiler might assume it should never inline #[cold] functions. In here we definitely want it to be inlined (returning a constant is much cheaper than calling, even tail-calling, a function), we just want the if-branch's contents to move to the end of the function.

}

let ptr = self.buf.as_mut_ptr();
let cap = self.capacity();

unsafe {
let value = T::from_tuple(T::TupleRepr::read(ptr, index, cap));

let src = T::TupleRepr::get_pointers(ptr, len - 1, cap);
let dst = T::TupleRepr::get_pointers(ptr, index, cap);
T::TupleRepr::copy(src, dst, 1);

self.buf.set_len(len - 1);

Ok(value)
}
}
}

impl<T: SoAble> Drop for SoAVec<T> {
Expand Down