public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Mathys Gasnier <mathys35.gasnier@gmail.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] rust: locks: Add `get_mut` method to `Lock`
Date: Sun, 25 Feb 2024 11:56:42 -0800	[thread overview]
Message-ID: <Zdubehd0Qsr96dgq@boqun-archlinux> (raw)
In-Reply-To: <CAAZKF4B_YKZg+S=e7jnWDvcKJOXEJ_E0MRNmzEzEa013XZrYzw@mail.gmail.com>

On Sun, Feb 25, 2024 at 10:21:23AM +0100, Mathys Gasnier wrote:
> Should i include this comment in this patch ?
> 

My suggestion is 1) in the comment of the `get_mut()` function, mention
that "`data` is not structurally pinned, so return a `&mut T` here" and
2) in the function body of `get_mut()`, at the safety comments, you only
need to put the reasoning explaining that `self` wouldn't get moved via
the return value of `self.get_unchecked_mut()`.

With these (along with the period and newline added), it'll be good to
me.

Regards,
Boqun

> Le ven. 23 févr. 2024 à 11:49, Alice Ryhl <aliceryhl@google.com> a écrit :
> 
> > On Fri, Feb 23, 2024 at 3:52 AM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > BTW, while we are at it, I think we should document the
> > > "structural/non-structural pinning" design decisions somewhere, for
> > > example in the struct definition:
> > >
> > >         #[pin_data]
> > >         pub struct Lock<T: ?Sized, B: Backend> {
> > >             ...
> > >             /// The data protected by the lock.
> > >             /// This field is non-structural pinned.
> > >             pub(crate) data: UnsafeCell<T>,
> > >         }
> > >
> > > Thoughts? Or do we think "non-structural pinned" should be the default
> > > case so no need to document it? I want to have a clear document for each
> > > field to avoid the accidental "everyone forgets what's the decision
> > > here" ;-)
> >
> > I would normally assume that "field is not marked #[pin]" implies that
> > it's not structurally pinned. But it could still be worth to call out
> > here.
> >
> > I prefer the wording "not structurally pinnned" over "non-structural
> > pinned".
> >
> > Alice
> >

      parent reply	other threads:[~2024-02-25 19:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-22 16:26 [PATCH v3] rust: locks: Add `get_mut` method to `Lock` Mathys-Gasnier via B4 Relay
2024-02-22 18:04 ` Martin Rodriguez Reboredo
2024-02-23  2:52 ` Boqun Feng
2024-02-23 10:49   ` Alice Ryhl
     [not found]     ` <CAAZKF4B_YKZg+S=e7jnWDvcKJOXEJ_E0MRNmzEzEa013XZrYzw@mail.gmail.com>
2024-02-25 19:56       ` Boqun Feng [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zdubehd0Qsr96dgq@boqun-archlinux \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathys35.gasnier@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=wedsonaf@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox