Re: [PATCH v4] rust: add global lock support

[Alice Ryhl]

On Thu, Oct 10, 2024 at 3:55 PM Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> On Thu, Oct 10, 2024 at 12:53:00PM +0200, Alice Ryhl wrote:
> [...]
> > > > +#[macro_export]
> > > > +macro_rules! global_lock {
> > > > +    {
> > > > +        $(#[$meta:meta])* $pub:vis static $name:ident: $kind:ident<$valuety:ty> = unsafe { uninit };
> > > > +        value: $value:expr;
> > >
> > > I would find it more natural to use `=` instead of `:` here, since then
> > > it would read as a normal statement with the semicolon at the end.
> > > Another alternative would be to use `,` instead of `;`, but that doesn't
> > > work nicely with the static keyword above (although you could make the
> > > user write it in another {}, but that also isn't ideal...).
> > >
> > > Using `=` instead of `:` makes my editor put the correct amount of
> > > indentation there, `:` adds a lot of extra spaces.
> >
> > That seems sensible.
> >
>
> While we are at it, how about we make the syntax:
>
>         global_lock!{
>             static MY_LOCK: Mutex<u32> = unsafe { 0 };
>         }
>
> or
>
>         global_lock!{
>             static MY_LOCK: Mutex<u32> = unsafe { uninit { 0 } };
>         }
>
> ?
>
> i.e. instead of a "value" field, we put it in the "initialization
> expression". To me, this make it more clear that "value" is the
> initialized value protected by the lock. Thoughts?

`uninit { 0 }` looks pretty terrible IMO. Can we come up with something better?

> Besides, instead of a "guard" type name, could you make a
> generic guard type over the "locked_by" type? E.g.
>
>         struct GlobalGuard<L: GlobalLockedBy>(Guard<...>, PhantomData<*mut L>);
>
> I feel like this could make the relationship between the guard type and
> the locked_by type more obvious. But maybe there's something I'm
> missing?

Sorry, I don't understand this. Why is the LockedBy type relevant to the guard?

Alice