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

jos
2024-12-11T17:05:44Z

> Can't we use the same warning message as Guard?

> #[must_use = "the lock unlocks immediately when the guard is unused"]

That is what I originally had in my first patch and how I understood
the annotation should be phrased, but @Boqun Feng maybe you can chime
in with your input?

>> + /// Note that `T` is `#[must_use]` but `Option<T>` is not.
>
> Should this be a comment, i.e. `//`, rather than part of the documentation?

Agreed, after reading more documentation conventions
(https://github.com/rust-lang/rfcs/blob/master/text/1574-more-api-documentation-conventions.md),
this should not be part of the rustdocs.

> I would also reword to explain the "what", not just "why", e.g. something like:
>
> // `Option<T>` is not `#[must_use]` even if `T` is, thus the
> attribute is needed here.

Thank you for the insight, I can see how there is missing clarity in
the current comment, will update in the next patch.

Thanks,
Jason

On Wed, Dec 11, 2024 at 1:45 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Wed, Dec 11, 2024 at 3:05 AM Jason Devers <dev.json2@gmail.com> wrote:
> >
> > + /// Note that `T` is `#[must_use]` but `Option<T>` is not.
>
> Should this be a comment, i.e. `//`, rather than part of the documentation?
>
> I would also reword to explain the "what", not just "why", e.g. something like:
>
> // `Option<T>` is not `#[must_use]` even if `T` is, thus the
> attribute is needed here.
>
> Cheers,
> Miguel