[WIP] Experiment with a safe adoption API

src/adopt.rs

@@ -1,6 +1,8 @@
+use core::fmt;
 use core::ptr;
 
 use crate::link::Link;
+use crate::trace::Trace;
 use crate::Rc;
 
 mod sealed {
@@ -12,6 +14,21 @@ mod sealed {
     impl<T> Sealed for Rc<T> {}
 }
 
+/// The error type for `try_adopt` methods.
+///
+/// See [`Adopt::try_adopt`] for more information.
+#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
+pub struct AdoptError(());
+
+#[cfg(feature = "std")]
+impl std::error::Error for AdoptError {}
+
+impl fmt::Display for AdoptError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str("Rc adoption failed because the Rc does not own a pointer to the adoptee")
+    }
+}
+
 /// Build a graph of linked [`Rc`] smart pointers to enable busting cycles on
 /// drop.
 ///
@@ -33,7 +50,23 @@ mod sealed {
 ///
 /// [`adopt_unchecked`]: Adopt::adopt_unchecked
 /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
-pub unsafe trait Adopt: sealed::Sealed {
+pub unsafe trait Adopt: sealed::Sealed + Sized {
+    /// The smart pointer's inner owned value.
+    type Inner;
+
+    /// TODO: document me!
+    fn adopt(this: &mut Self, other: &Self) -> Self
+    where
+        Self::Inner: Trace,
+    {
+        Self::try_adopt(this, other).unwrap_or_else(|err| panic!("{}", err))
+    }
+
+    /// TODO: document me!
+    fn try_adopt(this: &mut Self, other: &Self) -> Result<Self, AdoptError>
+    where
+        Self::Inner: Trace;
+
     /// Perform bookkeeping to record that `this` has an owned reference to
     /// `other`.
     ///
@@ -78,6 +111,50 @@ pub unsafe trait Adopt: sealed::Sealed {
 /// Implementation of [`Adopt`] for [`Rc`] which enables `Rc`s to form a cycle
 /// of strong references that are reaped by `Rc`'s [`Drop`] implementation.
 unsafe impl<T> Adopt for Rc<T> {
+    /// `Rc`'s inner owned value. For an `Rc<T>`, the inner owned value is a
+    /// `T`.
+    type Inner = T;
+
+    /// TODO: document me!
+    fn try_adopt(this: &mut Self, other: &Self) -> Result<Self, AdoptError>
+    where
+        Self::Inner: Trace,
+    {
+        // Use internal iteration on `this`'s owned `Rc`s to look for `other`.
+        //
+        // If `this` can yield a mutable reference to an `Rc` that has the same
+        // inner allocation as `other`, we can safely assert that `this` owns
+        // an `Rc` with the same `RcBox` as `other`.
+        let needle = other.inner() as *const _;
+        let mut found = None;
+        Trace::yield_owned_rcs(this.as_ref(), |node| {
+            if found.is_some() {
+                return;
+            }
+            // If `this` yields an `Rc` with the same `RcBox` as the given
+            // `other`, `this` owns a `Rc` that points to the same allocation as
+            // `other`, which fulfills the safety invariant of `adopt_unchecked`.
+            if ptr::eq(needle, node.inner()) {
+                // Clone the node we've found that matches the needle so we can
+                // save a reference to the `RcBox` we want to adopt.
+                found = Some(Rc::clone(node));
+            }
+        });
+        if let Some(node) = found {
+            // SAFETY: `yield_owned_rcs` yielded a mutable reference to an `Rc`
+            // matching `other`'s inner allocation, which means `this` must own
+            // an `Rc` matching `other`.
+            //
+            // This uphold's adopt_unchecked's safety invariant.
+            unsafe {
+                Self::adopt_unchecked(this, &node);
+            }
+            Ok(node)
+        } else {
+            Err(AdoptError(()))
+        }
+    }
+
     /// Perform bookkeeping to record that `this` has an owned reference to
     /// `other`.
     ///

src/lib.rs

@@ -139,6 +139,7 @@ mod drop;
 mod hash;
 mod link;
 mod rc;
+mod trace;
 
 // Doc modules
 #[cfg(any(doctest, docsrs))]
@@ -147,8 +148,10 @@ mod rc;
 pub mod implementing_self_referential_data_structures;
 
 pub use adopt::Adopt;
+pub use adopt::AdoptError;
 pub use rc::Rc;
 pub use rc::Weak;
+pub use trace::Trace;
 
 /// Cactus alias for [`Rc`].
 pub type CactusRef<T> = Rc<T>;

src/trace.rs

@@ -0,0 +1,9 @@
+use crate::rc::Rc;
+
+/// TODO: document me!
+pub trait Trace: Sized {
+    /// TODO: document me!
+    fn yield_owned_rcs<F>(&self, mark: F)
+    where
+        F: FnMut(&mut Rc<Self>);
+}

tests/leak_doubly_linked_list.rs

@@ -1,84 +1,120 @@
 #![warn(clippy::all)]
 #![warn(clippy::pedantic)]
 #![allow(clippy::shadow_unrelated)]
+#![forbid(unsafe_code)]
 
-use cactusref::{Adopt, Rc};
+use cactusref::{Adopt, Rc, Trace};
 use core::cell::RefCell;
 use core::iter;
+use core::ops::Deref;
+
+struct NodeCell<T>(RefCell<Node<T>>);
+
+impl<T> NodeCell<T> {
+    fn new(data: T) -> Self {
+        Self(RefCell::new(Node {
+            prev: None,
+            next: None,
+            data,
+        }))
+    }
+}
+
+impl<T> Deref for NodeCell<T> {
+    type Target = RefCell<Node<T>>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
 
 struct Node<T> {
-    pub prev: Option<Rc<RefCell<Self>>>,
-    pub next: Option<Rc<RefCell<Self>>>,
+    pub prev: Option<Rc<NodeCell<T>>>,
+    pub next: Option<Rc<NodeCell<T>>>,
     pub data: T,
 }
 
+impl<T> Trace for NodeCell<T> {
+    fn yield_owned_rcs<F>(&self, mut mark: F)
+    where
+        F: FnMut(&mut Rc<Self>),
+    {
+        if let Some(ref mut prev) = self.borrow_mut().prev {
+            mark(prev);
+        }
+        if let Some(ref mut next) = self.borrow_mut().next {
+            mark(next);
+        }
+    }
+}
+
 struct List<T> {
-    pub head: Option<Rc<RefCell<Node<T>>>>,
+    pub head: Option<Rc<NodeCell<T>>>,
 }
 
 impl<T> List<T> {
-    fn pop(&mut self) -> Option<Rc<RefCell<Node<T>>>> {
+    fn pop(&mut self) -> Option<Rc<NodeCell<T>>> {
         let head = self.head.take()?;
-        let tail = head.borrow_mut().prev.take();
-        let next = head.borrow_mut().next.take();
-        if let Some(ref tail) = tail {
+        let mut tail = head.borrow_mut().prev.take();
+        let mut next = head.borrow_mut().next.take();
+
+        if let Some(ref mut tail) = tail {
             Rc::unadopt(&head, tail);
             Rc::unadopt(tail, &head);
 
             tail.borrow_mut().next = next.as_ref().map(Rc::clone);
             if let Some(ref next) = next {
-                unsafe {
-                    Rc::adopt_unchecked(tail, next);
-                }
+                Rc::adopt(tail, next);
             }
         }
-        if let Some(ref next) = next {
+
+        if let Some(ref mut next) = next {
             Rc::unadopt(&head, next);
             Rc::unadopt(next, &head);
 
             next.borrow_mut().prev = tail.as_ref().map(Rc::clone);
             if let Some(ref tail) = tail {
-                unsafe {
-                    Rc::adopt_unchecked(next, tail);
-                }
+                Rc::adopt(next, tail);
             }
         }
+
         self.head = next;
         Some(head)
     }
 }
 
 impl<T> From<Vec<T>> for List<T> {
     fn from(list: Vec<T>) -> Self {
-        let nodes = list
+        if list.is_empty() {
+            return Self { head: None };
+        }
+        let mut nodes = list
             .into_iter()
-            .map(|data| {
-                Rc::new(RefCell::new(Node {
-                    prev: None,
-                    next: None,
-                    data,
-                }))
-            })
+            .map(|data| Rc::new(NodeCell::new(data)))
             .collect::<Vec<_>>();
+
         for i in 0..nodes.len() - 1 {
-            let curr = &nodes[i];
-            let next = &nodes[i + 1];
-            curr.borrow_mut().next = Some(Rc::clone(next));
-            next.borrow_mut().prev = Some(Rc::clone(curr));
-            unsafe {
-                Rc::adopt_unchecked(curr, next);
-                Rc::adopt_unchecked(next, curr);
-            }
-        }
-        let tail = &nodes[nodes.len() - 1];
-        let head = &nodes[0];
-        tail.borrow_mut().next = Some(Rc::clone(head));
-        head.borrow_mut().prev = Some(Rc::clone(tail));
-        unsafe {
-            Rc::adopt_unchecked(tail, head);
-            Rc::adopt_unchecked(head, tail);
+            let next = Rc::clone(&nodes[i + 1]);
+            let curr = &mut nodes[i];
+            curr.borrow_mut().next = Some(Rc::clone(&next));
+            Rc::adopt(curr, &next);
+
+            let curr = Rc::clone(&nodes[i]);
+            let next = &mut nodes[i + 1];
+            next.borrow_mut().prev = Some(Rc::clone(&curr));
+            Rc::adopt(next, &curr);
         }
 
+        let head = Rc::clone(&nodes[0]);
+        let tail = nodes.last_mut().unwrap();
+        tail.borrow_mut().next = Some(Rc::clone(&head));
+        Rc::adopt(tail, &head);
+
+        let tail = Rc::clone(&nodes[nodes.len() - 1]);
+        let head = &mut nodes[0];
+        head.borrow_mut().prev = Some(Rc::clone(&tail));
+        Rc::adopt(head, &tail);
+
         let head = Rc::clone(head);
         Self { head: Some(head) }
     }
@@ -95,10 +131,12 @@ fn leak_doubly_linked_list() {
         .take(10)
         .collect::<Vec<_>>();
     let mut list = List::from(list);
+
     let head = list.pop().unwrap();
     assert!(head.borrow().data.starts_with('a'));
     assert_eq!(Rc::strong_count(&head), 1);
     assert_eq!(list.head.as_ref().map(Rc::strong_count), Some(3));
+
     let weak = Rc::downgrade(&head);
     drop(head);
     assert!(weak.upgrade().is_none());