Re: [PATCH v14 1/7] rust: sync: add `OnceLock`

[Andreas Hindborg <a.hindborg@kernel.org>]

"Wren Turkal" <wt@penguintechs.org> writes:

> On 7/2/25 6:18 AM, Andreas Hindborg wrote:

[...]

>> +pub struct OnceLock<T> {
>> +    init: Atomic<u32>,
>> +    value: Opaque<T>,
>> +}
>
> This type looks very much like the Once type in rust's stdlib. I am
> wondering if the api could be changed to match that api. I know that
> this type is trying to provide a version subset of std::sync::OnceLock
> that doesn't allow resetting the type like these apis:
>
> * https://doc.rust-lang.org/std/sync/struct.OnceLock.html#method.get_mut
> * https://doc.rust-lang.org/std/sync/struct.OnceLock.html#method.take
>
> However, these methods can only be used on mut. See here for failing
> example:
> https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=a78e51203c5b9555e3c151e162f0acab
>
> I think it might make more sense to match the api of the stdlib API and
> maybe only implement the methods you need.

I agree, it would be nice to match the names to std. But I do not like
that they have `OnceLock::set`, `OnceLock::try_init` and
`OnceLock::get_or{_try}_init`. Why is it not `OnceLock::init` or
`OnceLock::try_set`?

>
>> +
>> +impl<T> Default for OnceLock<T> {
>> +    fn default() -> Self {
>> +        Self::new()
>> +    }
>> +}
>
> Any reason not to use #[derive(Default)]?

We don't have `Default` for neither `Atomic` or `Opaque`.

[...]

> Might also be worth implementing get_or_{try,}init, which get the value
> while initializing.

I did not have a user for those, so not adding for now. It would be dead
code. They should be fairly straight forward to add though.

>
>> +        // INVARIANT: We obtain exclusive access to the contained allocation and write 1 to
>> +        // `init`.
>> +        if let Ok(0) = self.init.cmpxchg(0, 1, Acquire) {
>> +            // SAFETY: We obtained exclusive access to the contained object.
>> +            unsafe { core::ptr::write(self.value.get(), value) };
>> +            // INVARIANT: We release our exclusive access and transition the object to shared
>> +            // access.
>> +            self.init.store(2, Release);
>> +            true
>> +        } else {
>> +            false
>> +        }
>> +    }
>> +}
>> +
>> +impl<T: Copy> OnceLock<T> {
>> +    /// Get a copy of the contained object.
>> +    ///
>> +    /// Returns [`None`] if the [`OnceLock`] is empty.
>> +    pub fn copy(&self) -> Option<T> {
>
> No equivalent in OnceLock. Similar to something like this:
>
> x.get().copied().unwrap(); // x is a OnceLock
>
> Example:
> https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f21068e55f73722544fb5ad341bce1c5
>
> Maybe not specifically needed?

I don't actually have a user for this, so I think I will drop it.


Best regards,
Andreas Hindborg