Re: [PATCH v6 14/26] rust: alloc: implement `IntoIterator` for `Vec`

[Benno Lossin <benno.lossin@proton.me>]

On 11.09.24 01:39, Danilo Krummrich wrote:
> On Tue, Sep 10, 2024 at 08:04:27PM +0000, Benno Lossin wrote:
>> On 16.08.24 02:10, Danilo Krummrich wrote:
>>> +/// [`IntoIterator`] trait).
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// ```
>>> +/// let v = kernel::kvec![0, 1, 2]?;
>>> +/// let iter = v.into_iter();
>>> +///
>>> +/// # Ok::<(), Error>(())
>>> +/// ```
>>> +pub struct IntoIter<T, A: Allocator> {
>>> +    ptr: *mut T,
>>> +    buf: NonNull<T>,
>>
>> No invariants for these two fields?
> 
> Suggestions?

When determining the invariants, I look at the places where you would
want to use them, ie the `SAFETY` comments that use these fields:
- for `buf` you only use it to free the backing allocation, so you only
  need that it has been allocated by `A` if `cap != 0`.
- for `ptr` you need that it is valid for reads for `size_of::<T>() *
  length` bytes.

So I would put those two things into invariants.

>>> +    len: usize,
>>> +    cap: usize,
>>> +    _p: PhantomData<A>,
>>> +}
>>> +
>>> +impl<T, A> IntoIter<T, A>
>>> +where
>>> +    A: Allocator,
>>> +{
>>> +    fn as_raw_mut_slice(&mut self) -> *mut [T] {
>>> +        ptr::slice_from_raw_parts_mut(self.ptr, self.len)
>>> +    }
>>> +}
>>> +
>>> +impl<T, A> Iterator for IntoIter<T, A>
>>> +where
>>> +    A: Allocator,
>>> +{
>>> +    type Item = T;
>>> +
>>> +    /// # Examples
>>> +    ///
>>> +    /// ```
>>> +    /// let v = kernel::kvec![1, 2, 3]?;
>>> +    /// let mut it = v.into_iter();
>>> +    ///
>>> +    /// assert_eq!(it.next(), Some(1));
>>> +    /// assert_eq!(it.next(), Some(2));
>>> +    /// assert_eq!(it.next(), Some(3));
>>> +    /// assert_eq!(it.next(), None);
>>> +    ///
>>> +    /// # Ok::<(), Error>(())
>>> +    /// ```
>>
>> AFAIK documentation on functions in trait implementations won't show up
>> in rustdoc (I just checked this). So I would remove it.
> 
> They don't, but the KUnit tests are still executed. :)

Oh I see, then may I suggest moving them to the module documentation or
put them onto `Vec`, that way people can also read them :)

---
Cheers,
Benno