Re: [PATCH v3 04/10] rust: list: add struct with prev/next pointers

[Alice Ryhl]

On Wed, Jul 31, 2024 at 8:41 PM Benno Lossin <benno.lossin@xxxxxxxxx> wrote:
>
> On 23.07.24 10:22, Alice Ryhl wrote:
> > +/// The prev/next pointers for an item in a linked list.
> > +///
> > +/// # Invariants
> > +///
> > +/// The fields are null if and only if this item is not in a list.
> > +#[repr(transparent)]
> > +pub struct ListLinks<const ID: u64 = 0> {
> > +    #[allow(dead_code)]
> > +    inner: Opaque<ListLinksFields>,
>
> Do you really need `Opaque`? Or would `UnsafeCell` be enough? (If it is
> enough and you change this, be aware that `Opaque` is `!Unpin`, so if
> you intend for `ListLinks` to also be `!Unpin`, then you need a
> `PhantomPinned`)

I need the `!Unpin` part for aliasing.

> > +}
> > +
> > +// SAFETY: The next/prev fields of a ListLinks can be moved across thread boundaries.
>
> Why? This is not a justification.

What would you say?

> > +unsafe impl<const ID: u64> Send for ListLinks<ID> {}
> > +// SAFETY: The type is opaque so immutable references to a ListLinks are useless. Therefore, it's
> > +// okay to have immutable access to a ListLinks from several threads at once.
>
> You don't need to argue via `Opaque`, the type doesn't expose any
> `&self` functions, so there are no functions to consider.

I'm not arguing via the `Opaque` type. I'm just using "opaque" as a
normal english word with its normal meaning.

Alice