additions based on integration into builtin-actors

filecoin-project · Apr 22, 2022 · 8a68e6a · 8a68e6a
1 parent bafcdc5
commit 8a68e6a
Show file tree

Hide file tree

Showing 9 changed files with 92 additions and 21 deletions.
diff --git a/fvm/src/state_tree.rs b/fvm/src/state_tree.rs
@@ -504,7 +504,7 @@ where
     where
         F: FnMut(Address, &ActorState) -> anyhow::Result<()>,
     {
-        self.hamt.for_each(|k, v| {
+        self.hamt.try_for_each(|k, v| {
             let addr = Address::from_bytes(&k.0)?;
             f(addr, v)
         })?;

diff --git a/ipld/amt/benches/amt_benchmark.rs b/ipld/amt/benches/amt_benchmark.rs
@@ -118,7 +118,7 @@ fn for_each(c: &mut Criterion) {
         b.iter(|| {
             let a = Amt::load(&cid, &db).unwrap();
             black_box(a)
-                .for_each(|_, _v: &u64| Ok::<_, ()>(()))
+                .try_for_each(|_, _v: &u64| Ok::<_, ()>(()))
                 .unwrap();
         })
     });

diff --git a/ipld/amt/src/amt.rs b/ipld/amt/src/amt.rs
@@ -284,26 +284,42 @@ where
     /// map.set(4, "Four".to_owned()).unwrap();
     ///
     /// let mut values: Vec<(u64, String)> = Vec::new();
-    /// map.for_each(|i, v| {
+    /// map.try_for_each(|i, v| {
     ///    values.push((i, v.clone()));
     ///    Ok::<_, ()>(())
     /// }).unwrap();
     /// assert_eq!(&values, &[(1, "One".to_owned()), (4, "Four".to_owned())]);
     /// ```
     #[inline]
-    pub fn for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
+    pub fn try_for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         F: FnMut(u64, &V) -> Result<(), U>,
     {
-        self.for_each_while(|i, x| {
+        self.try_for_each_while(|i, x| {
             f(i, x)?;
             Ok(true)
         })
     }
 
+    #[inline]
+    pub fn for_each<F>(&self, mut f: F) -> Result<(), Error<BS::Error>>
+    where
+        V: DeserializeOwned,
+        F: FnMut(u64, &V),
+    {
+        self.try_for_each(|k, v| {
+            f(k, v);
+            Ok(())
+        })
+        .map_err(|err| match err {
+            EitherError::User(()) => unreachable!(),
+            EitherError::Amt(e) => e,
+        })
+    }
+
     /// Iterates over each value in the Amt and runs a function on the values, for as long as that
     /// function keeps returning `true`.
-    pub fn for_each_while<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
+    pub fn try_for_each_while<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         F: FnMut(u64, &V) -> Result<bool, U>,
     {
@@ -319,22 +335,51 @@ where
             .map(|_| ())
     }
 
+    /// Iterates over each value in the Amt and runs a function on the values, for as long as that
+    /// function keeps returning `true`.
+    pub fn for_each_while<F>(&self, mut f: F) -> Result<(), Error<BS::Error>>
+    where
+        F: FnMut(u64, &V) -> bool,
+    {
+        self.try_for_each_while::<_, ()>(|key, value| Ok(f(key, value)))
+            .map_err(|err| match err {
+                EitherError::User(()) => unreachable!(),
+                EitherError::Amt(e) => e,
+            })
+    }
+
     /// Iterates over each value in the Amt and runs a function on the values that allows modifying
     /// each value.
-    pub fn for_each_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
+    pub fn try_for_each_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         V: Clone,
         F: FnMut(u64, &mut ValueMut<'_, V>) -> Result<(), U>,
     {
-        self.for_each_while_mut(|i, x| {
+        self.try_for_each_while_mut(|i, x| {
             f(i, x)?;
             Ok(true)
         })
     }
 
+    /// Iterates over each value in the Amt and runs a function on the values that allows modifying
+    /// each value.
+    pub fn for_each_mut<F>(&mut self, mut f: F) -> Result<(), Error<BS::Error>>
+    where
+        V: Clone,
+        F: FnMut(u64, &mut ValueMut<'_, V>),
+    {
+        self.for_each_while_mut(|i, x| {
+            f(i, x);
+            true
+        })
+    }
+
     /// Iterates over each value in the Amt and runs a function on the values that allows modifying
     /// each value, for as long as that function keeps returning `true`.
-    pub fn for_each_while_mut<F, U>(&mut self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
+    pub fn try_for_each_while_mut<F, U>(
+        &mut self,
+        mut f: F,
+    ) -> Result<(), EitherError<U, BS::Error>>
     where
         // TODO remove clone bound when go-interop doesn't require it.
         // (If needed without, this bound can be removed by duplicating function signatures)
@@ -392,4 +437,20 @@ where
             Ok(())
         }
     }
+
+    /// Iterates over each value in the Amt and runs a function on the values that allows modifying
+    /// each value, for as long as that function keeps returning `true`.
+    pub fn for_each_while_mut<F>(&mut self, mut f: F) -> Result<(), Error<BS::Error>>
+    where
+        // TODO remove clone bound when go-interop doesn't require it.
+        // (If needed without, this bound can be removed by duplicating function signatures)
+        V: Clone,
+        F: FnMut(u64, &mut ValueMut<'_, V>) -> bool,
+    {
+        self.try_for_each_while_mut::<_, ()>(|key, value| Ok(f(key, value)))
+            .map_err(|err| match err {
+                EitherError::User(()) => unreachable!(),
+                EitherError::Amt(e) => e,
+            })
+    }
 }
diff --git a/ipld/amt/src/lib.rs b/ipld/amt/src/lib.rs
@@ -13,7 +13,7 @@ mod root;
 mod value_mut;
 
 pub use self::amt::Amt;
-pub use self::error::Error;
+pub use self::error::{EitherError, Error};
 pub(crate) use self::node::Node;
 pub(crate) use self::root::Root;
 pub use self::value_mut::ValueMut;

diff --git a/ipld/amt/tests/amt_tests.rs b/ipld/amt/tests/amt_tests.rs
@@ -322,7 +322,6 @@ fn for_each() {
     let mut x = 0;
     a.for_each(|_, _: &BytesDe| {
         x += 1;
-        Ok::<(), ()>(())
     })
     .unwrap();
 
@@ -343,13 +342,12 @@ fn for_each() {
                 );
             }
             x += 1;
-            Ok::<(), ()>(())
         })
         .unwrap();
     assert_eq!(x, indexes.len());
 
     // Iteration again will be read diff with go-interop, since they do not cache
-    new_amt.for_each(|_, _: &BytesDe| Ok::<(), ()>(())).unwrap();
+    new_amt.for_each(|_, _: &BytesDe| {}).unwrap();
 
     assert_eq!(
         c.to_string().as_str(),
@@ -385,7 +383,6 @@ fn for_each_mutate() {
                 // Value it's set to doesn't matter, just cloning for expedience
                 **v = v.clone();
             }
-            Ok::<(), ()>(())
         })
         .unwrap();
 

diff --git a/ipld/blockstore/src/lib.rs b/ipld/blockstore/src/lib.rs
@@ -14,7 +14,7 @@ pub use block::*;
 ///
 /// The cgo blockstore adapter implements this trait.
 pub trait Blockstore {
-    type Error: std::error::Error + std::fmt::Debug + Send + Sync + 'static;
+    type Error: std::error::Error + Send + Sync + 'static;
 
     /// Gets the block from the blockstore.
     fn get(&self, k: &Cid) -> Result<Option<Vec<u8>>, Self::Error>;

diff --git a/ipld/hamt/benches/hamt_benchmark.rs b/ipld/hamt/benches/hamt_benchmark.rs
@@ -95,7 +95,7 @@ fn for_each(c: &mut Criterion) {
         b.iter(|| {
             let a = Hamt::<_, _>::load(&cid, &db).unwrap();
             black_box(a)
-                .for_each(|_k, _v: &BenchData| Ok::<_, ()>(()))
+                .try_for_each(|_k, _v: &BenchData| Ok::<_, ()>(()))
                 .unwrap();
         })
     });

diff --git a/ipld/hamt/src/hamt.rs b/ipld/hamt/src/hamt.rs
@@ -304,21 +304,37 @@ where
     /// map.set(4, 2).unwrap();
     ///
     /// let mut total = 0;
-    /// map.for_each(|_, v: &u64| {
+    /// map.try_for_each(|_, v: &u64| {
     ///    total += v;
     ///    Ok::<(), ()>(())
     /// }).unwrap();
     /// assert_eq!(total, 3);
     /// ```
     #[inline]
-    pub fn for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
+    pub fn try_for_each<F, U>(&self, mut f: F) -> Result<(), EitherError<U, BS::Error>>
     where
         V: DeserializeOwned,
         F: FnMut(&K, &V) -> Result<(), U>,
     {
         self.root.for_each(self.store.borrow(), &mut f)
     }
 
+    #[inline]
+    pub fn for_each<F>(&self, mut f: F) -> Result<(), Error<BS::Error>>
+    where
+        V: DeserializeOwned,
+        F: FnMut(&K, &V),
+    {
+        self.try_for_each(|k, v| {
+            f(k, v);
+            Ok(())
+        })
+        .map_err(|err| match err {
+            EitherError::User(()) => unreachable!(),
+            EitherError::Hamt(e) => e,
+        })
+    }
+
     /// Consumes this HAMT and returns the Blockstore it owns.
     pub fn into_store(self) -> BS {
         self.store

diff --git a/ipld/hamt/tests/hamt_tests.rs b/ipld/hamt/tests/hamt_tests.rs
@@ -273,7 +273,6 @@ fn for_each() {
     hamt.for_each(|k, v| {
         assert_eq!(k, v);
         count += 1;
-        Ok::<(), ()>(())
     })
     .unwrap();
     assert_eq!(count, 200);
@@ -291,7 +290,6 @@ fn for_each() {
     hamt.for_each(|k, v| {
         assert_eq!(k, v);
         count += 1;
-        Ok::<(), ()>(())
     })
     .unwrap();
     assert_eq!(count, 200);
@@ -301,7 +299,6 @@ fn for_each() {
     hamt.for_each(|k, v| {
         assert_eq!(k, v);
         count += 1;
-        Ok::<(), ()>(())
     })
     .unwrap();
     assert_eq!(count, 200);