From cb88ca8aa68d860a0cfe3b738f511b7af463d0eb Mon Sep 17 00:00:00 2001 From: Lukas Bergdoll Date: Fri, 6 Sep 2024 17:49:15 +0200 Subject: [PATCH] Add support for mut ref dependents This has feature that has been requested multiple times by users, https://github.com/Voultapher/self_cell/pull/36 and https://github.com/Voultapher/self_cell/issues/59. --- README.md | 4 +- examples/Cargo.toml | 3 +- examples/REAME.md | 2 + .../mut_ref_to_owner_in_builder/Cargo.toml | 10 + .../mut_ref_to_owner_in_builder/README.md | 14 ++ .../mut_ref_to_owner_in_builder/src/main.rs | 25 ++ examples/owner_with_lifetime/Cargo.toml | 4 +- src/lib.rs | 57 ++++- src/unsafe_self_cell.rs | 124 ++++++++++ tests/self_cell.rs | 218 +++++++++++++++++- 10 files changed, 453 insertions(+), 8 deletions(-) create mode 100644 examples/mut_ref_to_owner_in_builder/Cargo.toml create mode 100644 examples/mut_ref_to_owner_in_builder/README.md create mode 100644 examples/mut_ref_to_owner_in_builder/src/main.rs diff --git a/README.md b/README.md index 9c20bbc..1db3b34 100644 --- a/README.md +++ b/README.md @@ -44,8 +44,8 @@ impl Debug for NewStructName { ... } Self-referential structs are currently not supported with safe vanilla Rust. The only reasonable safe alternative is to expect the user to juggle 2 separate data -structures which is a mess. The library solution ouroboros is really expensive -to compile due to its use of procedural macros. +structures which is a mess. The library solution ouroboros is expensive to +compile due to its use of procedural macros. This alternative is `no_std`, uses no proc-macros, some self contained unsafe and works on stable Rust, and is miri tested. With a total of less than 300 diff --git a/examples/Cargo.toml b/examples/Cargo.toml index 8e6f9ba..470940e 100644 --- a/examples/Cargo.toml +++ b/examples/Cargo.toml @@ -3,4 +3,5 @@ members = [ "fallible_dependent_construction", "lazy_ast", "owner_with_lifetime", -] \ No newline at end of file + "mut_ref_to_owner_in_builder", +] diff --git a/examples/REAME.md b/examples/REAME.md index a3119c1..c9ef37a 100644 --- a/examples/REAME.md +++ b/examples/REAME.md @@ -6,5 +6,7 @@ The advanced examples: - [How to build a lazy AST with self_cell](lazy_ast) +- [How to handle dependents that take a mutable reference](mut_ref_to_owner_in_builder) + - [How to use an owner type with lifetime](owner_with_lifetime) diff --git a/examples/mut_ref_to_owner_in_builder/Cargo.toml b/examples/mut_ref_to_owner_in_builder/Cargo.toml new file mode 100644 index 0000000..e6c8f97 --- /dev/null +++ b/examples/mut_ref_to_owner_in_builder/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "owner_with_lifetime" +version = "0.1.0" +authors = ["Lukas Bergdoll "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +self_cell = { path="../../"} \ No newline at end of file diff --git a/examples/mut_ref_to_owner_in_builder/README.md b/examples/mut_ref_to_owner_in_builder/README.md new file mode 100644 index 0000000..e30a012 --- /dev/null +++ b/examples/mut_ref_to_owner_in_builder/README.md @@ -0,0 +1,14 @@ +# `mut_ref_to_owner_in_builder` Example + +This example shows how to handle dependent types that want to reference the +owner by `&mut`. This works by using the wrapper type `MutBorrow` around the +owner type. This allows us to call `borrow_mut` in the builder function. This +example also shows how to recover the owner value if desired. + +Run this example with `cargo run`, it should output: + +``` +dependent before pop: abc +dependent after pop: ab +recovered owner: ab +``` \ No newline at end of file diff --git a/examples/mut_ref_to_owner_in_builder/src/main.rs b/examples/mut_ref_to_owner_in_builder/src/main.rs new file mode 100644 index 0000000..d90ad9d --- /dev/null +++ b/examples/mut_ref_to_owner_in_builder/src/main.rs @@ -0,0 +1,25 @@ +use self_cell::{self_cell, MutBorrow}; + +type MutStringRef<'a> = &'a mut String; + +self_cell!( + struct MutStringCell { + owner: MutBorrow, + + #[covariant] + dependent: MutStringRef, + } +); + +fn main() { + let mut cell = MutStringCell::new(MutBorrow::new("abc".into()), |owner| owner.borrow_mut()); + + cell.with_dependent_mut(|_owner, dependent| { + println!("dependent before pop: {}", dependent); + dependent.pop(); + println!("dependent after pop: {}", dependent); + }); + + let recovered_owner: String = cell.into_owner().into_inner(); + println!("recovered owner: {}", recovered_owner); +} diff --git a/examples/owner_with_lifetime/Cargo.toml b/examples/owner_with_lifetime/Cargo.toml index e6c8f97..153aa28 100644 --- a/examples/owner_with_lifetime/Cargo.toml +++ b/examples/owner_with_lifetime/Cargo.toml @@ -1,5 +1,5 @@ [package] -name = "owner_with_lifetime" +name = "mut_ref_to_owner_in_builder" version = "0.1.0" authors = ["Lukas Bergdoll "] edition = "2018" @@ -7,4 +7,4 @@ edition = "2018" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -self_cell = { path="../../"} \ No newline at end of file +self_cell = { path = "../../" } diff --git a/src/lib.rs b/src/lib.rs index a0a060d..6074f97 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,8 +41,8 @@ //! //! Self-referential structs are currently not supported with safe vanilla Rust. //! The only reasonable safe alternative is to have the user juggle 2 separate -//! data structures which is a mess. The library solution ouroboros is really -//! expensive to compile due to its use of procedural macros. +//! data structures which is a mess. The library solution ouroboros is expensive +//! to compile due to its use of procedural macros. //! //! This alternative is `no_std`, uses no proc-macros, some self contained //! unsafe and works on stable Rust, and is miri tested. With a total of less @@ -138,6 +138,8 @@ //! - [How to build a lazy AST with //! self_cell](https://github.com/Voultapher/self_cell/tree/main/examples/lazy_ast) //! +//! - [How to handle dependents that take a mutable reference](https://github.com/Voultapher/self_cell/tree/main/examples/mut_ref_to_owner_in_builder) see also [`MutBorrow`] +//! //! - [How to use an owner type with //! lifetime](https://github.com/Voultapher/self_cell/tree/main/examples/owner_with_lifetime) //! @@ -352,6 +354,9 @@ macro_rules! self_cell { ) -> Self { use ::core::ptr::NonNull; + #[allow(unused)] + use $crate::unsafe_self_cell::MutBorrowDefaultTrait; + unsafe { // All this has to happen here, because there is not good way // of passing the appropriate logic into UnsafeSelfCell::new @@ -380,6 +385,8 @@ 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); @@ -388,6 +395,8 @@ macro_rules! self_cell { dependent_ptr.write(dependent_builder(&*owner_ptr)); ::core::mem::forget(drop_guard); + $crate::_mut_borrow_lock!(owner_ptr, $Owner); + Self { unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new( joined_void_ptr, @@ -407,6 +416,9 @@ macro_rules! self_cell { ) -> ::core::result::Result { use ::core::ptr::NonNull; + #[allow(unused)] + use $crate::unsafe_self_cell::MutBorrowDefaultTrait; + unsafe { // See fn new for more explanation. @@ -425,6 +437,8 @@ 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); @@ -434,6 +448,8 @@ macro_rules! self_cell { dependent_ptr.write(dependent); ::core::mem::forget(drop_guard); + $crate::_mut_borrow_lock!(owner_ptr, $Owner); + ::core::result::Result::Ok(Self { unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new( joined_void_ptr, @@ -456,6 +472,9 @@ macro_rules! self_cell { ) -> ::core::result::Result { use ::core::ptr::NonNull; + #[allow(unused)] + use $crate::unsafe_self_cell::MutBorrowDefaultTrait; + unsafe { // See fn new for more explanation. @@ -474,6 +493,8 @@ 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); @@ -483,6 +504,8 @@ macro_rules! self_cell { dependent_ptr.write(dependent); ::core::mem::forget(drop_guard); + $crate::_mut_borrow_lock!(owner_ptr, $Owner); + ::core::result::Result::Ok(Self { unsafe_self_cell: $crate::unsafe_self_cell::UnsafeSelfCell::new( joined_void_ptr, @@ -671,3 +694,33 @@ macro_rules! _impl_automatic_derive { )); }; } + +#[doc(hidden)] +#[macro_export] +macro_rules! _mut_borrow_unlock { + ($owner_ptr:expr, $Owner:ty) => {{ + let wrapper = std::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 { + ($owner_ptr:expr, $Owner:ty) => {{ + let wrapper = std::mem::transmute::< + &$Owner, + &$crate::unsafe_self_cell::MutBorrowSpecWrapper<$Owner>, + >(&*$owner_ptr); + + // If `T` is `MutBorrow` will call `lock`, otherwise a no-op. + wrapper.lock(); + }}; +} + +pub use unsafe_self_cell::MutBorrow; diff --git a/src/unsafe_self_cell.rs b/src/unsafe_self_cell.rs index 8a429ba..895a1f5 100644 --- a/src/unsafe_self_cell.rs +++ b/src/unsafe_self_cell.rs @@ -1,5 +1,6 @@ #![allow(clippy::needless_lifetimes)] +use core::cell::{Cell, UnsafeCell}; use core::marker::PhantomData; use core::mem; use core::ptr::{drop_in_place, read, NonNull}; @@ -223,3 +224,126 @@ impl JoinedCell { (owner_ptr, dependent_ptr) } } + +/// Wrapper type that allows creating a self-referential type that hold a mutable borrow `&mut T`. +/// +/// Example usage: +/// +/// ``` +/// use self_cell::{self_cell, MutBorrow}; +/// +/// type MutStringRef<'a> = &'a mut String; +/// +/// self_cell!( +/// struct MutStringCell { +/// owner: MutBorrow, +/// +/// #[covariant] +/// dependent: MutStringRef, +/// } +/// ); +/// +/// let mut cell = MutStringCell::new(MutBorrow::new("abc".into()), |owner| owner.borrow_mut()); +/// cell.with_dependent_mut(|_owner, dependent| { +/// assert_eq!(dependent, &"abc"); +/// dependent.pop(); +/// assert_eq!(dependent, &"ab"); +/// }); +/// +/// let recovered_owner: String = cell.into_owner().into_inner(); +/// assert_eq!(recovered_owner, "ab"); +/// ``` +pub struct MutBorrow { + // Private on purpose. + is_locked: Cell, + value: UnsafeCell, +} + +impl MutBorrow { + /// Constructs a new `MutBorrow`. + pub fn new(value: T) -> Self { + Self { + is_locked: Cell::new(true), + value: UnsafeCell::new(value), + } + } + + /// Obtains a mutable reference to the underlying data. + /// + /// This function can only sensibly be used in the builder function. Afterwards, it's impossible + /// to access the inner value, with the exception of [`MutBorrow::into_inner`]. + /// + /// # Panics + /// + /// Will panic if called anywhere but in the dependent constructor. Will also panic if called + /// more than once. + pub fn borrow_mut(&self) -> &mut T { + if self.is_locked.get() { + panic!("Tried to access locked MutBorrow") + } else { + // 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. + unsafe { &mut *self.value.get() } + } + } + + /// Consumes `self` and returns the wrapped value. + pub fn into_inner(self) -> T { + self.value.into_inner() + } +} + +// 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) {} +} + +unsafe impl MutBorrowDefaultTrait for T {} + +#[doc(hidden)] +#[repr(transparent)] +pub struct MutBorrowSpecWrapper(T); + +impl MutBorrowSpecWrapper { + #[doc(hidden)] + pub unsafe fn unlock(&mut self) { + ::unlock(&mut self.0); + } + + #[doc(hidden)] + pub fn lock(&self) { + ::lock(&self.0); + } +} + +// 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 MutBorrowSpecImpl for MutBorrow { + // 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); + } +} diff --git a/tests/self_cell.rs b/tests/self_cell.rs index f7a2338..2413daf 100644 --- a/tests/self_cell.rs +++ b/tests/self_cell.rs @@ -10,7 +10,7 @@ use std::str; use once_cell::unsync::OnceCell; -use self_cell::self_cell; +use self_cell::{self_cell, MutBorrow}; #[derive(Debug, Eq, PartialEq)] pub struct Ast<'input>(pub Vec<&'input str>); @@ -237,6 +237,7 @@ fn catch_panic_in_from() { #[derive(Debug, Clone, PartialEq)] struct Owner(String); + #[allow(dead_code)] #[derive(Debug)] struct PanicCtor<'a>(&'a i32); @@ -351,6 +352,7 @@ fn drop_order() { } struct Owner(Rc>>); + #[allow(dead_code)] struct Dependent<'a>(&'a Owner, Rc>>); impl Drop for Owner { @@ -688,6 +690,7 @@ fn result_name_hygiene() { #[allow(dead_code)] type Result = std::result::Result; + #[allow(dead_code)] type VecRef<'a> = &'a Vec; self_cell!( @@ -778,3 +781,216 @@ fn cell_mem_size() { assert_eq!(size_of::(), size_of::<*const u8>()); assert_eq!(size_of::>(), size_of::<*const u8>()); } + +#[test] +#[should_panic] +fn mut_borrow_new_borrow() { + let mut_borrow = MutBorrow::new(45); + let _ = mut_borrow.borrow_mut(); +} + +#[test] +fn mut_borrow_mutate() { + let mut mut_borrow = MutBorrow::new(45); + + unsafe { + let wrapper = std::mem::transmute::< + &mut MutBorrow, + &mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper>, + >(&mut mut_borrow); + + wrapper.unlock(); + } + + let mut_ref: &mut i32 = mut_borrow.borrow_mut(); + + *mut_ref = 789; + + assert_eq!(mut_borrow.into_inner(), 789); +} + +#[test] +#[should_panic] +fn mut_borrow_double_borrow() { + let mut mut_borrow = MutBorrow::new(45); + + unsafe { + let wrapper = std::mem::transmute::< + &mut MutBorrow, + &mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper>, + >(&mut mut_borrow); + + wrapper.unlock(); + } + + let _mut_ref_a: &mut i32 = mut_borrow.borrow_mut(); + let _mut_ref_b: &mut i32 = mut_borrow.borrow_mut(); +} + +#[test] +#[should_panic] +fn mut_borrow_lock_borrow() { + let mut mut_borrow = MutBorrow::new(45); + + unsafe { + let wrapper = std::mem::transmute::< + &mut MutBorrow, + &mut self_cell::unsafe_self_cell::MutBorrowSpecWrapper>, + >(&mut mut_borrow); + + wrapper.unlock(); + } + + { + let wrapper = unsafe { + std::mem::transmute::< + &MutBorrow, + &self_cell::unsafe_self_cell::MutBorrowSpecWrapper>, + >(&mut_borrow) + }; + + wrapper.lock(); + } + + let _: &mut i32 = mut_borrow.borrow_mut(); +} + +type MutStringRef<'a> = &'a mut String; + +self_cell!( + struct MutStringCell { + owner: MutBorrow, + + #[covariant] + dependent: MutStringRef, + } +); + +#[test] +fn mut_borrow_new() { + let mut cell = MutStringCell::new(MutBorrow::new("abc".into()), |owner| owner.borrow_mut()); + + let _ = cell.borrow_owner(); + + assert_eq!(cell.borrow_dependent(), &"abc"); + + cell.with_dependent(|_owner, dependent| { + assert_eq!(dependent, &"abc"); + }); + + cell.with_dependent_mut(|_owner, dependent| { + assert_eq!(dependent, &"abc"); + dependent.pop(); + assert_eq!(dependent, &"ab"); + }); + + assert_eq!(cell.into_owner().into_inner(), "ab"); +} + +#[test] +fn mut_borrow_try_new() { + let mut cell = MutStringCell::try_new( + MutBorrow::new("abc".into()), + |owner| -> Result<&mut String, ()> { std::result::Result::Ok(owner.borrow_mut()) }, + ) + .unwrap(); + + cell.with_dependent_mut(|_owner, dependent| { + dependent.pop(); + }); + + assert_eq!(cell.into_owner().into_inner(), "ab"); + + let res = MutStringCell::try_new(MutBorrow::new("wxyz".into()), |_owner| { + std::result::Result::Err(678) + }); + let err: i32 = match res { + std::result::Result::Ok(_) => { + unreachable!() + } + std::result::Result::Err(err) => err, + }; + + assert_eq!(err, 678); +} + +#[test] +fn mut_borrow_try_new_or_recover() { + let mut cell = MutStringCell::try_new_or_recover( + MutBorrow::new("abc".into()), + |owner| -> Result<&mut String, ()> { std::result::Result::Ok(owner.borrow_mut()) }, + ) + .map_err(|(_owner, err)| err) + .unwrap(); + + cell.with_dependent_mut(|_owner, dependent| { + dependent.pop(); + }); + + assert_eq!(cell.into_owner().into_inner(), "ab"); + + let res = MutStringCell::try_new_or_recover(MutBorrow::new("wxyz".into()), |_owner| { + std::result::Result::Err(678) + }); + let (owner, err): (MutBorrow, i32) = match res { + std::result::Result::Ok(_) => { + unreachable!() + } + std::result::Result::Err(err) => err, + }; + + assert_eq!(owner.into_inner(), "wxyz"); + assert_eq!(err, 678); +} + +type MutBorrowStringRef<'a> = &'a MutBorrow; + +self_cell!( + struct MutStringFullBorrowCell { + owner: MutBorrow, + + #[covariant] + dependent: MutBorrowStringRef, + } +); + +#[test] +#[should_panic] +fn mut_borrow_new_borrow_mut() { + let cell = MutStringCell::new(MutBorrow::new("abc".into()), |owner| owner.borrow_mut()); + + cell.borrow_owner().borrow_mut(); +} + +#[test] +#[should_panic] +fn mut_borrow_new_late_borrow_mut() { + let cell = MutStringFullBorrowCell::new(MutBorrow::new("abc".into()), |owner| owner); + + cell.borrow_owner().borrow_mut(); +} + +#[test] +#[should_panic] +fn mut_borrow_try_new_late_borrow_mut() { + let cell = MutStringFullBorrowCell::try_new( + MutBorrow::new("abc".into()), + |owner| -> Result { std::result::Result::Ok(owner) }, + ) + .unwrap(); + + cell.borrow_owner().borrow_mut(); +} + +#[test] +#[should_panic] +fn mut_borrow_try_new_or_recover_late_borrow_mut() { + let cell = MutStringFullBorrowCell::try_new_or_recover( + MutBorrow::new("abc".into()), + |owner| -> Result { std::result::Result::Ok(owner) }, + ) + .map_err(|(_owner, err)| err) + .unwrap(); + + cell.borrow_owner().borrow_mut(); +}