From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: rust: sync: lock: Add Lock::get_mut_pinned() Date: Wed, 27 May 2026 14:07:41 +1000 Message-ID: In-Reply-To: <20260526205419.1055109-2-lyude@redhat.com> References: <20260526205419.1055109-1-lyude@redhat.com> <20260526205419.1055109-2-lyude@redhat.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review This patch adds a method to `Lock` that provides `Pin<&mut T>` access when = you hold a `Pin<&mut Lock<...>>`, bypassing the actual locking since `&mut`= guarantees exclusivity at compile time. This mirrors `std::sync::Mutex::ge= t_mut()`. **Correctness**: The implementation is sound. `Lock::data` is declared as `= #[pin] pub(crate) data: UnsafeCell` (lock.rs:118-119), so `T` is structu= rally pinned within the `Lock`. Having `Pin<&mut Self>` guarantees exclusiv= e access, and returning `Pin<&mut T>` preserves the pin projection correctl= y. The `map_unchecked_mut` + `UnsafeCell::get_mut` path is the standard pat= tern. **Minor items**: 1. The SAFETY comment is slightly sparse: ```rust // SAFETY: We return a pinned T, ensuring we don't move T. unsafe { self.map_unchecked_mut(|data| data.data.get_mut()) } ``` It would be more precise to note *why* this is a valid pin projection =E2= =80=94 i.e., that `data` is structurally pinned within `Lock` (via `#[pin]`= on the field). The current comment explains the output constraint (we don'= t move T) but not the input justification (the field is structurally pinned= ). This is minor since the `#[pin]` attribute is visible on the struct defi= nition. 2. The `#[inline(always)]` is arguably aggressive for a method that will li= kely only be called in `Drop` paths and similar low-frequency situations. `= #[inline]` would be sufficient, consistent with the existing `lock()` and `= try_lock()` methods in the same `impl` block. 3. The commit message references "These two functions" (plural), but only o= ne function is added. Looks like the commit message may have been written w= hen a non-pinned `get_mut()` was also planned. **Verdict**: Good. The approach is correct and matches standard Rust practi= ce. --- --- Generated by Claude Code Patch Reviewer