Re: [PATCH] rust: sync: add #[must_use] to Lock::try_lock

jos
2024-12-09T17:09:44Z

> I use aliceryhl@google.com for kernel contributions.

Sorry about this, I will update with the correct email!

> Please use the version option of `format-patch`, e.g. `-v4`, when you
> send new versions, so that they are tagged as such. Otherwise, all the
> patches look like they are the first version, which is confusing for
> tooling (and people :)

> Also, in general, it is best to wait a day or two to give people a
> chance to review your patches, so that you do not need to accumulate
> too many versions.

Will do! I really appreciate the patience and feedback, please
continue doing so :)

> + #[must_use = "if unused, leading to an empty critical section"]

> I am not a native speaker, but this sounds a bit strange. Is it
> written like this due to how it gets "rendered" by the compiler later
> on?

I agree this sounds a bit strange, perhaps it would sound more proper to say:

`#[must_use = "if unused, leads to an empty critical section"]`

Will give it some time to receive more thoughts before sending an
updated version.

Thanks,
Jason

On Mon, Dec 9, 2024 at 3:58 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sun, Dec 8, 2024 at 11:26 PM Jason Devers <dev.json2@gmail.com> wrote:
> >
> > The `Lock::try_lock` function returns an `Option<Guard<...>>`, but it
> > currently does not issue a warning if the return value is unused.
> > To avoid potential bugs, the `#[must_use]` annotation is added to ensure proper usage.
> >
> > Note that `T` is `#[must_use]` but `Option<T>` is not.
> > For more context, see: https://github.com/rust-lang/rust/issues/71368.
> >
> > Suggested-by: Alice Ryhl <alice@ryhl.io>
> > Link: https://github.com/Rust-for-Linux/linux/issues/1133
> > Signed-off-by: Jason Devers <dev.json2@gmail.com>
>
> Please use the version option of `format-patch`, e.g. `-v4`, when you
> send new versions, so that they are tagged as such. Otherwise, all the
> patches look like they are the first version, which is confusing for
> tooling (and people :)
>
> Also, in general, it is best to wait a day or two to give people a
> chance to review your patches, so that you do not need to accumulate
> too many versions.
>
> > + #[must_use = "if unused, leading to an empty critical section"]
>
> I am not a native speaker, but this sounds a bit strange. Is it
> written like this due to how it gets "rendered" by the compiler later
> on?
>
> Thanks!
>
> Cheers,
> Miguel