feat!: remove clone restriction on capsules (#15)

Fixes #12

rearch/src/capsule_reader.rs

@@ -21,19 +21,33 @@ impl<'scope, 'total> CapsuleReader<'scope, 'total> {
         Self(InternalCapsuleReader::Normal { id, txn })
     }
 
-    /// Reads the current data of the supplied capsule, initializing it if needed.
+    /// Returns a clone of the current data of the supplied capsule, initializing it if needed.
     /// Internally forms a dependency graph amongst capsules, so feel free to conditionally invoke
     /// this function in case you only conditionally need a capsule's data.
     ///
     /// # Panics
     /// Panics when a capsule attempts to read itself in its first build,
     /// or when a mocked [`CapsuleReader`] attempts to read a capsule's data that wasn't mocked.
-    pub fn get<C: Capsule>(&mut self, capsule: C) -> C::Data {
+    pub fn get<C: Capsule>(&mut self, capsule: C) -> C::Data
+    where
+        C::Data: Clone,
+    {
+        self.as_ref(capsule).clone()
+    }
+
+    /// Returns a ref to the current data of the supplied capsule, initializing it if needed.
+    /// Internally forms a dependency graph amongst capsules, so feel free to conditionally invoke
+    /// this function in case you only conditionally need a capsule's data.
+    ///
+    /// # Panics
+    /// Panics when a capsule attempts to read itself in its first build,
+    /// or when a mocked [`CapsuleReader`] attempts to read a capsule's data that wasn't mocked.
+    pub fn as_ref<C: Capsule>(&mut self, capsule: C) -> &C::Data {
         match &mut self.0 {
             InternalCapsuleReader::Normal { ref id, txn } => {
                 let (this, other) = (id, capsule.id());
                 if this == &other {
-                    return txn.try_read(&capsule).unwrap_or_else(|| {
+                    return txn.try_read_ref(&capsule).unwrap_or_else(|| {
                         let name = std::any::type_name::<C>();
                         panic!(
                             "{name} ({id:?}) tried to read itself on its first build! {} {} {}",
@@ -44,40 +58,44 @@ impl<'scope, 'total> CapsuleReader<'scope, 'total> {
                     });
                 }
 
-                // Adding dep relationship is after read_or_init to ensure manager is initialized
-                let data = txn.read_or_init(capsule);
-                txn.add_dependency_relationship(other, this);
-                data
+                txn.ensure_initialized(capsule);
+                txn.add_dependency_relationship(Arc::clone(&other), this);
+                txn.try_read_ref_raw::<C>(&other)
+                    .expect("Ensured capsule was initialized above")
             }
             InternalCapsuleReader::Mock { mocks } => {
                 let id = capsule.id();
-                #[allow(clippy::map_unwrap_or)] // suggestion is ugly/hard to read
-                mocks
-                    .get(&id)
-                    .map(crate::downcast_capsule_data::<C>)
-                    .map(dyn_clone::clone)
-                    .unwrap_or_else(|| {
+                mocks.get(&id).map_or_else(
+                    || {
                         panic!(
                             "Mock CapsuleReader was used to read {} ({id:?}) {}",
                             std::any::type_name::<C>(),
                             "when it was not included in the mock!"
                         );
-                    })
+                    },
+                    crate::downcast_capsule_data::<C>,
+                )
             }
         }
     }
 }
 
 #[cfg(feature = "better-api")]
-impl<A: Capsule> FnOnce<(A,)> for CapsuleReader<'_, '_> {
+impl<A: Capsule> FnOnce<(A,)> for CapsuleReader<'_, '_>
+where
+    A::Data: Clone,
+{
     type Output = A::Data;
     extern "rust-call" fn call_once(mut self, args: (A,)) -> Self::Output {
         self.call_mut(args)
     }
 }
 
 #[cfg(feature = "better-api")]
-impl<A: Capsule> FnMut<(A,)> for CapsuleReader<'_, '_> {
+impl<A: Capsule> FnMut<(A,)> for CapsuleReader<'_, '_>
+where
+    A::Data: Clone,
+{
     extern "rust-call" fn call_mut(&mut self, args: (A,)) -> Self::Output {
         self.get(args.0)
     }

rearch/src/lib.rs

@@ -1,6 +1,5 @@
 #![cfg_attr(feature = "better-api", feature(unboxed_closures, fn_traits))]
 
-use dyn_clone::DynClone;
 use std::{
     any::Any,
     cell::OnceCell,
@@ -29,14 +28,12 @@ pub use txn::*;
 // - `Send` is required because `CapsuleManager` needs to store a copy of the capsule
 // - `'static` is required to store a copy of the capsule, and for TypeId::of()
 pub trait Capsule: Send + 'static {
-    /// The type of data associated with this capsule.
-    /// Capsule types must be `Clone + Send + Sync + 'static` (see [`CapsuleData`]).
-    /// It is recommended to only put types with "cheap" clones in Capsules;
-    /// think Copy types, small Vecs and other containers, basic data structures, and Arcs.
-    /// If you are dealing with a bigger chunk of data, consider wrapping it in an [`Arc`].
-    /// Note: The `im` crate plays *very nicely* with rearch.
+    /// The type of data associated with this capsule, which must be `Send + Sync + 'static`.
+    /// Capsule data that implements `Clone` will also unlock a few convenience methods.
+    /// Note: when your types do implement `Clone`, it is suggested to be a "cheap" Clone.
+    /// `Arc`s, small collections/data structures, and the `im` crate are great for this.
     // Associated type so that Capsule can only be implemented once for each concrete type
-    type Data: CapsuleData;
+    type Data: Send + Sync + 'static;
 
     /// Builds the capsule's immutable data using a given snapshot of the data flow graph.
     /// (The snapshot, a `ContainerWriteTxn`, is abstracted away for you via [`CapsuleHandle`].)
@@ -63,7 +60,7 @@ pub trait Capsule: Send + 'static {
 }
 impl<T, F> Capsule for F
 where
-    T: CapsuleData,
+    T: Send + Sync + 'static,
     F: Fn(CapsuleHandle) -> T + Send + 'static,
 {
     type Data = T;
@@ -79,17 +76,9 @@ where
     }
 }
 
-/// Represents the type of a capsule's data;
-/// Capsules' data must be `Clone + Send + Sync + 'static`.
-/// You seldom need to reference this in your application's code;
-/// you are probably looking for [`CData`] instead.
-pub trait CapsuleData: Any + DynClone + Send + Sync + 'static {}
-impl<T: Clone + Send + Sync + 'static> CapsuleData for T {}
-dyn_clone::clone_trait_object!(CapsuleData);
-
 /// Shorthand for `Clone + Send + Sync + 'static`,
 /// which makes returning `impl Trait` far easier from capsules,
-/// where `Trait` is often a `Fn(Foo) -> Bar`.
+/// where `Trait` is often an `Fn` from side effects.
 pub trait CData: Clone + Send + Sync + 'static {}
 impl<T: Clone + Send + Sync + 'static> CData for T {}
 
@@ -180,7 +169,7 @@ impl Container {
         self.0.with_write_txn(rebuilder, to_run)
     }
 
-    /// Performs a *consistent* read on all supplied capsules.
+    /// Performs a *consistent* read on all supplied capsules that have cloneable data.
     ///
     /// Consistency is important here: if you need the current data from a few different capsules,
     /// *do not* read them individually, but rather group them together with one `read()` call.
@@ -193,7 +182,7 @@ impl Container {
     /// Internally, tries to read all supplied capsules with a read txn first (cheap),
     /// but if that fails (i.e., capsules' data not present in the container),
     /// spins up a write txn and initializes all needed capsules (which blocks).
-    pub fn read<CL: CapsuleList>(&self, capsules: CL) -> CL::Data {
+    pub fn read<Capsules: CapsulesWithCloneRead>(&self, capsules: Capsules) -> Capsules::Data {
         capsules.read(self)
     }
 
@@ -278,17 +267,18 @@ impl Drop for ListenerHandle {
     }
 }
 
-/// A list of capsules.
-/// This is either a singular capsule, like `count`, or a tuple, like `(foo, bar)`.
-pub trait CapsuleList {
+/// A list of capsules with cloneable data.
+/// This is either a singular capsule, like `foo_capsule`,
+/// or a tuple, like `(foo_capsule, bar_capsule)`.
+pub trait CapsulesWithCloneRead {
     type Data;
     fn read(self, container: &Container) -> Self::Data;
 }
 macro_rules! generate_capsule_list_impl {
     ($($C:ident),+) => {
         paste::paste! {
             #[allow(non_snake_case, unused_parens)]
-            impl<$($C: Capsule),*> CapsuleList for ($($C),*) {
+            impl<$($C: Capsule),*> CapsulesWithCloneRead for ($($C),*) where $($C::Data: Clone),* {
                 type Data = ($($C::Data),*);
                 fn read(self, container: &Container) -> Self::Data {
                     let ($([<i $C>]),*) = self;
@@ -445,8 +435,7 @@ impl CapsuleManager {
             .remove(&id)
             .as_ref()
             .map(downcast_capsule_data::<C>)
-            .map(dyn_clone::clone)
-            .map_or(true, |old_data| !C::eq(&old_data, &new_data));
+            .map_or(true, |old_data| !C::eq(old_data, &new_data));
 
         txn.data.insert(id, Arc::new(new_data));
 

rearch/src/txn.rs

@@ -16,11 +16,18 @@ impl<'a> ContainerReadTxn<'a> {
 
 impl ContainerReadTxn<'_> {
     #[must_use]
-    pub fn try_read<C: Capsule>(&self, capsule: &C) -> Option<C::Data> {
+    pub fn try_read<C: Capsule>(&self, capsule: &C) -> Option<C::Data>
+    where
+        C::Data: Clone,
+    {
+        self.try_read_ref(capsule).map(Clone::clone)
+    }
+
+    #[must_use]
+    pub fn try_read_ref<C: Capsule>(&self, capsule: &C) -> Option<&C::Data> {
         self.data
             .get(&capsule.id())
             .map(crate::downcast_capsule_data::<C>)
-            .map(dyn_clone::clone)
     }
 }
 
@@ -46,31 +53,46 @@ impl<'a> ContainerWriteTxn<'a> {
 }
 
 impl ContainerWriteTxn<'_> {
+    pub fn read_or_init<C: Capsule>(&mut self, capsule: C) -> C::Data
+    where
+        C::Data: Clone,
+    {
+        self.read_or_init_ref(capsule).clone()
+    }
+
     #[allow(clippy::missing_panics_doc)] // false positive
-    pub fn read_or_init<C: Capsule>(&mut self, capsule: C) -> C::Data {
+    pub fn read_or_init_ref<C: Capsule>(&mut self, capsule: C) -> &C::Data {
         let id = capsule.id();
+        self.ensure_initialized(capsule);
+        self.try_read_ref_raw::<C>(&id)
+            .expect("Ensured capsule was initialized above")
+    }
+
+    #[must_use]
+    pub fn try_read<C: Capsule>(&self, capsule: &C) -> Option<C::Data>
+    where
+        C::Data: Clone,
+    {
+        self.try_read_ref::<C>(capsule).map(Clone::clone)
+    }
+
+    #[must_use]
+    pub fn try_read_ref<C: Capsule>(&self, capsule: &C) -> Option<&C::Data> {
+        self.try_read_ref_raw::<C>(&capsule.id())
+    }
 
+    pub(crate) fn try_read_ref_raw<C: Capsule>(&self, id: &Id) -> Option<&C::Data> {
+        self.data.get(id).map(crate::downcast_capsule_data::<C>)
+    }
+
+    pub(crate) fn ensure_initialized<C: Capsule>(&mut self, capsule: C) {
+        let id = capsule.id();
         if !self.data.contains_key(&id) {
             #[cfg(feature = "logging")]
             log::debug!("Initializing {} ({:?})", std::any::type_name::<C>(), id);
 
             self.build_capsule(capsule);
         }
-
-        self.try_read_raw::<C>(&id)
-            .expect("Data should be present due to checking/building capsule above")
-    }
-
-    #[must_use]
-    pub fn try_read<C: Capsule>(&self, capsule: &C) -> Option<C::Data> {
-        self.try_read_raw::<C>(&capsule.id())
-    }
-
-    fn try_read_raw<C: Capsule>(&self, id: &Id) -> Option<C::Data> {
-        self.data
-            .get(id)
-            .map(crate::downcast_capsule_data::<C>)
-            .map(dyn_clone::clone)
     }
 
     /// Forcefully disposes only the requested node, cleaning up the node's direct dependencies.