From 8a5cfddec71a1b88dbb0dc9a377ab9db4eb501c9 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Wed, 17 Jul 2024 11:00:58 -0700 Subject: [PATCH 1/4] Fix GcCellRef::map doctest Signed-off-by: Anders Kaseorg --- gc/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gc/src/lib.rs b/gc/src/lib.rs index 17ef897..28f0e08 100644 --- a/gc/src/lib.rs +++ b/gc/src/lib.rs @@ -747,7 +747,7 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { } } - /// Makes a new `GcCellRef` from a component of the borrowed data. + /// Makes a new `GcCellRef` for a component of the borrowed data. /// /// The `GcCell` is already immutably borrowed, so this cannot fail. /// @@ -763,7 +763,7 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { /// let c = GcCell::new((5, 'b')); /// let b1: GcCellRef<(u32, char)> = c.borrow(); /// let b2: GcCellRef = GcCellRef::map(b1, |t| &t.0); - /// //assert_eq!(b2, 5); + /// assert_eq!(*b2, 5); /// ``` #[inline] pub fn map(orig: Self, f: F) -> GcCellRef<'a, U> From d25e341eab65f53740f098af67843c23b07d5276 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Wed, 17 Jul 2024 11:54:05 -0700 Subject: [PATCH 2/4] Simplify with ManuallyDrop Signed-off-by: Anders Kaseorg --- gc/src/lib.rs | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/gc/src/lib.rs b/gc/src/lib.rs index 28f0e08..249f214 100644 --- a/gc/src/lib.rs +++ b/gc/src/lib.rs @@ -16,7 +16,7 @@ use std::cmp::Ordering; use std::fmt::{self, Debug, Display}; use std::hash::{Hash, Hasher}; use std::marker::PhantomData; -use std::mem; +use std::mem::{self, ManuallyDrop}; use std::ops::{Deref, DerefMut}; use std::ptr::{self, NonNull}; use std::rc::Rc; @@ -194,9 +194,7 @@ impl Gc { /// unsafe { Gc::from_raw(x_ptr) }; /// ``` pub fn into_raw(this: Self) -> *const T { - let ptr: *const T = GcBox::value_ptr(this.inner_ptr()); - mem::forget(this); - ptr + GcBox::value_ptr(ManuallyDrop::new(this).inner_ptr()) } /// Constructs an `Gc` from a raw pointer. @@ -771,16 +769,16 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { U: ?Sized, F: FnOnce(&T) -> &U, { - let ret = GcCellRef { - flags: orig.flags, - value: f(orig.value), - }; + let value = f(orig.value); // We have to tell the compiler not to call the destructor of GcCellRef, // because it will update the borrow flags. - std::mem::forget(orig); + let orig = ManuallyDrop::new(orig); - ret + GcCellRef { + flags: orig.flags, + value, + } } /// Splits a `GcCellRef` into multiple `GcCellRef`s for different components of the borrowed data. @@ -812,7 +810,11 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { orig.flags.set(orig.flags.get().add_reading()); - let ret = ( + // We have to tell the compiler not to call the destructor of GcCellRef, + // because it will update the borrow flags. + let orig = ManuallyDrop::new(orig); + + ( GcCellRef { flags: orig.flags, value: a, @@ -821,13 +823,7 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { flags: orig.flags, value: b, }, - ); - - // We have to tell the compiler not to call the destructor of GcCellRef, - // because it will update the borrow flags. - std::mem::forget(orig); - - ret + ) } } From d50f6c01a23a4ff8583ce8b02aecf32190f0a9da Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Wed, 17 Jul 2024 11:01:09 -0700 Subject: [PATCH 3/4] Add GcCellRef::filter_map Fixes #176. Signed-off-by: Anders Kaseorg --- gc/src/lib.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/gc/src/lib.rs b/gc/src/lib.rs index 249f214..90bb28e 100644 --- a/gc/src/lib.rs +++ b/gc/src/lib.rs @@ -781,6 +781,47 @@ impl<'a, T: ?Sized> GcCellRef<'a, T> { } } + /// Makes a new `GcCellRef` for an optional component of the borrowed data. + /// The original guard is returned as an `Err(..)` if the closure returns + /// `None`. + /// + /// The `GcCell` is already immutably borrowed, so this cannot fail. + /// + /// This is an associated function that needs to be used as + /// `GcCellRef::filter_map(...)`. A method would interfere with methods of + /// the same name on the contents of a `GcCellRef` used through `Deref`. + /// + /// # Examples + /// + /// ``` + /// use gc::{GcCell, GcCellRef}; + /// + /// let c = GcCell::new(vec![1, 2, 3]); + /// let b1: GcCellRef> = c.borrow(); + /// let b2: Result, _> = GcCellRef::filter_map(b1, |v| v.get(1)); + /// assert_eq!(*b2.unwrap(), 2); + /// ``` + #[inline] + pub fn filter_map(orig: Self, f: F) -> Result, Self> + where + U: ?Sized, + F: FnOnce(&T) -> Option<&U>, + { + match f(orig.value) { + None => Err(orig), + Some(value) => { + // We have to tell the compiler not to call the destructor of GcCellRef, + // because it will update the borrow flags. + let orig = ManuallyDrop::new(orig); + + Ok(GcCellRef { + flags: orig.flags, + value, + }) + } + } + } + /// Splits a `GcCellRef` into multiple `GcCellRef`s for different components of the borrowed data. /// /// The `GcCell` is already immutably borrowed, so this cannot fail. From 2bbea85805094eaaa10de2e7ad51a2ab1263c0e6 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Wed, 17 Jul 2024 11:01:19 -0700 Subject: [PATCH 4/4] Add GcCellRefMut::filter_map Signed-off-by: Anders Kaseorg --- gc/src/lib.rs | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/gc/src/lib.rs b/gc/src/lib.rs index 90bb28e..8d086cf 100644 --- a/gc/src/lib.rs +++ b/gc/src/lib.rs @@ -906,7 +906,7 @@ impl<'a, T: Trace + ?Sized, U: ?Sized> GcCellRefMut<'a, T, U> { /// Makes a new `GcCellRefMut` for a component of the borrowed data, e.g., an enum /// variant. /// - /// The `GcCellRefMut` is already mutably borrowed, so this cannot fail. + /// The `GcCell` is already mutably borrowed, so this cannot fail. /// /// This is an associated function that needs to be used as /// `GcCellRefMut::map(...)`. A method would interfere with methods of the same @@ -946,6 +946,55 @@ impl<'a, T: Trace + ?Sized, U: ?Sized> GcCellRefMut<'a, T, U> { value: f(value), } } + + /// Makes a new `GcCellRefMut` for an optional component of the borrowed + /// data. The original guard is returned as an `Err(..)` if the closure + /// returns `None`. + /// + /// The `GcCell` is already mutably borrowed, so this cannot fail. + /// + /// This is an associated function that needs to be used as + /// `GcCellRefMut::filter_map(...)`. A method would interfere with methods + /// of the same name on the contents of a `GcCell` used through `Deref`. + /// + /// # Examples + /// + /// ``` + /// use gc::{GcCell, GcCellRefMut}; + /// + /// let c = GcCell::new(vec![1, 2, 3]); + /// + /// { + /// let b1: GcCellRefMut> = c.borrow_mut(); + /// let mut b2: Result, u32>, _> = GcCellRefMut::filter_map(b1, |v| v.get_mut(1)); + /// + /// if let Ok(mut b2) = b2 { + /// *b2 += 2; + /// } + /// } + /// + /// assert_eq!(*c.borrow(), vec![1, 4, 3]); + /// ``` + #[inline] + pub fn filter_map(orig: Self, f: F) -> Result, Self> + where + V: ?Sized, + F: FnOnce(&mut U) -> Option<&mut V>, + { + let gc_cell = orig.gc_cell; + + // Use MaybeUninit to avoid calling the destructor of + // GcCellRefMut (which would update the borrow flags) and to + // avoid duplicating the mutable reference orig.value (which + // would be UB). + let orig = mem::MaybeUninit::new(orig); + let value = unsafe { ptr::addr_of!((*orig.as_ptr()).value).read() }; + + match f(value) { + None => Err(unsafe { orig.assume_init() }), + Some(value) => Ok(GcCellRefMut { gc_cell, value }), + } + } } impl<'a, T: Trace + ?Sized, U: ?Sized> Deref for GcCellRefMut<'a, T, U> {