Skip to content

Commit

Permalink
Model unlocked -> locked via unlocked by construction
Browse files Browse the repository at this point in the history 
This is less code, less run-time work and
conceptually harder to miss-use. Win-win.
  • Loading branch information
@Voultapher
Voultapher committed Sep 6, 2024
1 parent 1e33ae4 commit 95da867
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 74 deletions.
20 changes: 0 additions & 20 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,6 @@ macro_rules! self_cell {
// Move owner into newly allocated space.
owner_ptr.write(owner);

$crate::_mut_borrow_unlock!(owner_ptr, $Owner);

// Drop guard that cleans up should building the dependent panic.
let drop_guard =
$crate::unsafe_self_cell::OwnerAndCellDropGuard::new(joined_ptr);
Expand Down Expand Up @@ -437,8 +435,6 @@ macro_rules! self_cell {
// Move owner into newly allocated space.
owner_ptr.write(owner);

$crate::_mut_borrow_unlock!(owner_ptr, $Owner);

// Drop guard that cleans up should building the dependent panic.
let mut drop_guard =
$crate::unsafe_self_cell::OwnerAndCellDropGuard::new(joined_ptr);
Expand Down Expand Up @@ -493,8 +489,6 @@ macro_rules! self_cell {
// Move owner into newly allocated space.
owner_ptr.write(owner);

$crate::_mut_borrow_unlock!(owner_ptr, $Owner);

// Drop guard that cleans up should building the dependent panic.
let mut drop_guard =
$crate::unsafe_self_cell::OwnerAndCellDropGuard::new(joined_ptr);
Expand Down Expand Up @@ -695,20 +689,6 @@ macro_rules! _impl_automatic_derive {
};
}

#[doc(hidden)]
#[macro_export]
macro_rules! _mut_borrow_unlock {
($owner_ptr:expr, $Owner:ty) => {{
let wrapper = ::core::mem::transmute::<
&mut $Owner,
&mut $crate::unsafe_self_cell::MutBorrowSpecWrapper<$Owner>,
>(&mut *$owner_ptr);

// If `T` is `MutBorrow` will call `unlock`, otherwise a no-op.
wrapper.unlock();
}};
}

#[doc(hidden)]
#[macro_export]
macro_rules! _mut_borrow_lock {
Expand Down
27 changes: 6 additions & 21 deletions src/unsafe_self_cell.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,10 @@ pub struct MutBorrow<T> {
impl<T> MutBorrow<T> {
/// Constructs a new `MutBorrow`.
pub fn new(value: T) -> Self {
// Use the Rust type system to model an affine type that can only go from unlocked -> locked
// but never the other way around.
Self {
is_locked: Cell::new(true),
is_locked: Cell::new(false),
value: UnsafeCell::new(value),
}
}
Expand All @@ -284,9 +286,9 @@ impl<T> MutBorrow<T> {
// Ensure this function can only be called once.
self.is_locked.set(true);

// SAFETY: `self.is_locked` starts out as locked and can only be unlocked with `unsafe`
// functions, which guarantees that this function can only be called once. And the
// `self.value` being private ensures that there are no other references to it.
// SAFETY: `self.is_locked` starts out as locked and can never be unlocked again, which
// guarantees that this function can only be called once. And the `self.value` being
// private ensures that there are no other references to it.
unsafe { &mut *self.value.get() }
}
}
Expand All @@ -300,9 +302,6 @@ impl<T> MutBorrow<T> {
// Provides the no-op for types that are not `MutBorrow`.
#[doc(hidden)]
pub unsafe trait MutBorrowDefaultTrait {
#[doc(hidden)]
unsafe fn unlock(&mut self) {}

#[doc(hidden)]
fn lock(&self) {}
}
Expand All @@ -314,11 +313,6 @@ unsafe impl<T> MutBorrowDefaultTrait for T {}
pub struct MutBorrowSpecWrapper<T>(T);

impl<T: MutBorrowSpecImpl> MutBorrowSpecWrapper<T> {
#[doc(hidden)]
pub unsafe fn unlock(&mut self) {
<T as MutBorrowSpecImpl>::unlock(&mut self.0);
}

#[doc(hidden)]
pub fn lock(&self) {
<T as MutBorrowSpecImpl>::lock(&self.0);
Expand All @@ -328,20 +322,11 @@ impl<T: MutBorrowSpecImpl> MutBorrowSpecWrapper<T> {
// unsafe trait to make sure users can't impl this without unsafe.
#[doc(hidden)]
pub unsafe trait MutBorrowSpecImpl {
#[doc(hidden)]
unsafe fn unlock(&mut self);

#[doc(hidden)]
fn lock(&self);
}

unsafe impl<T> MutBorrowSpecImpl for MutBorrow<T> {
// SAFETY: The caller MUST only ever call this once.
#[doc(hidden)]
unsafe fn unlock(&mut self) {
self.is_locked.set(false);
}

#[doc(hidden)]
fn lock(&self) {
self.is_locked.set(true);
Expand Down
41 changes: 8 additions & 33 deletions tests/self_cell.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -783,24 +783,17 @@ fn cell_mem_size() {
}

#[test]
#[should_panic]
fn mut_borrow_new_borrow() {
let mut_borrow = MutBorrow::new(45);
let _ = mut_borrow.borrow_mut();
let mut_borrow = MutBorrow::new("abc".to_string());

let string_ref: &mut String = mut_borrow.borrow_mut();
string_ref.pop();
assert_eq!(string_ref, "ab");
}

#[test]
fn mut_borrow_mutate() {
let mut mut_borrow = MutBorrow::new(45);

unsafe {
let wrapper = std::mem::transmute::<
&mut MutBorrow<i32>,
&mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper<MutBorrow<i32>>,
>(&mut mut_borrow);

wrapper.unlock();
}
let mut_borrow = MutBorrow::new(45);

let mut_ref: &mut i32 = mut_borrow.borrow_mut();

Expand All @@ -812,16 +805,7 @@ fn mut_borrow_mutate() {
#[test]
#[should_panic]
fn mut_borrow_double_borrow() {
let mut mut_borrow = MutBorrow::new(45);

unsafe {
let wrapper = std::mem::transmute::<
&mut MutBorrow<i32>,
&mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper<MutBorrow<i32>>,
>(&mut mut_borrow);

wrapper.unlock();
}
let mut_borrow = MutBorrow::new(45);

let _mut_ref_a: &mut i32 = mut_borrow.borrow_mut();
let _mut_ref_b: &mut i32 = mut_borrow.borrow_mut();
Expand All @@ -830,16 +814,7 @@ fn mut_borrow_double_borrow() {
#[test]
#[should_panic]
fn mut_borrow_lock_borrow() {
let mut mut_borrow = MutBorrow::new(45);

unsafe {
let wrapper = std::mem::transmute::<
&mut MutBorrow<i32>,
&mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper<MutBorrow<i32>>,
>(&mut mut_borrow);

wrapper.unlock();
}
let mut_borrow = MutBorrow::new(45);

{
let wrapper = unsafe {
Expand Down

0 comments on commit 95da867

Please sign in to comment.