Replace Space::atom_iter() method by Space::visit()

c/src/space.rs

@@ -732,54 +732,24 @@ impl Space for CSpace {
             None
         }
     }
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        struct CSpaceIterator<'a>(&'a CSpace, *mut c_void);
-        impl<'a> Iterator for CSpaceIterator<'a> {
-            type Item = &'a Atom;
-            fn next(&mut self) -> Option<&'a Atom> {
-                let api = unsafe{ &*self.0.api };
-                if let Some(next_atom) = api.next_atom {
-                    let atom_ref = next_atom(&self.0.params, self.1);
+    fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
+        let api = unsafe{ &*self.api };
+        match (api.new_atom_iterator_state, api.next_atom) {
+            (Some(new_atom_iterator_state), Some(next_atom)) => {
+                let ctx = new_atom_iterator_state(&self.params);
+                loop {
+                    let atom_ref = next_atom(&self.params, ctx);
                     if atom_ref.is_null() {
-                        None
+                        break;
                     } else {
-                        Some(atom_ref.into_ref())
+                        v.accept(Cow::Borrowed(atom_ref.into_ref()));
                     }
-                } else {
-                    panic!("next_atom function must be implemented if new_atom_iterator_state is implemented");
-                }
-            }
-        }
-        impl Drop for CSpaceIterator<'_> {
-            fn drop(&mut self) {
-                let api = unsafe{ &*self.0.api };
-                if let Some(free_atom_iterator_state) = api.free_atom_iterator_state {
-                    free_atom_iterator_state(&self.0.params, self.1);
-                } else {
-                    panic!("free_atom_iterator_state function must be implemented if new_atom_iterator_state is implemented");
                 }
-            }
-        }
-
-        let api = unsafe{ &*self.api };
-        if let Some(new_atom_iterator_state) = api.new_atom_iterator_state {
-            let ctx = new_atom_iterator_state(&self.params);
-            if ctx.is_null() {
-                None
-            } else {
-                let new_iter = CSpaceIterator(self, ctx);
-                Some(Box::new(new_iter))
-            }
-        } else {
-            None
+                Ok(())
+            },
+            _ => Err(()),
         }
     }
-    fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
-        self.atom_iter().map_or(Err(()), |iter| {
-            iter.for_each(|atom| { v.accept(Cow::Borrowed(atom)) });
-            Ok(())
-        })
-    }
     fn as_any(&self) -> Option<&dyn std::any::Any> {
         Some(self)
     }

lib/src/metta/runner/stdlib/space.rs

@@ -260,6 +260,13 @@ mod tests {
         assert_eq!(result[2], vec![Atom::gnd(stdlib_space)]);
     }
 
+    fn collect_atoms(space: &dyn Space) -> Vec<Atom> {
+        let mut atoms = Vec::new();
+        space.visit(&mut |atom: std::borrow::Cow<Atom>| atoms.push(atom.into_owned()))
+            .expect("Space::visit is not implemented");
+        atoms
+    }
+
     #[test]
     fn remove_atom_op() {
         let space = DynSpace::new(metta_space("
@@ -270,7 +277,7 @@ mod tests {
         let res = RemoveAtomOp{}.execute(&mut vec![satom, expr!(("foo" "bar"))]).expect("No result returned");
         // REM: can return Bool in future
         assert_eq!(res, vec![UNIT_ATOM]);
-        let space_atoms: Vec<Atom> = space.borrow().as_space().atom_iter().unwrap().cloned().collect();
+        let space_atoms = collect_atoms(space.borrow().as_space());
         assert_eq_no_order!(space_atoms, vec![expr!(("bar" "foo"))]);
     }
 
@@ -282,7 +289,7 @@ mod tests {
         "));
         let satom = Atom::gnd(space.clone());
         let res = GetAtomsOp{}.execute(&mut vec![satom]).expect("No result returned");
-        let space_atoms: Vec<Atom> = space.borrow().as_space().atom_iter().unwrap().cloned().collect();
+        let space_atoms = collect_atoms(space.borrow().as_space());
         assert_eq_no_order!(res, space_atoms);
         assert_eq_no_order!(res, vec![expr!(("foo" "bar")), expr!(("bar" "foo"))]);
     }
@@ -292,7 +299,7 @@ mod tests {
         let res = NewSpaceOp{}.execute(&mut vec![]).expect("No result returned");
         let space = res.get(0).expect("Result is empty");
         let space = space.as_gnd::<DynSpace>().expect("Result is not space");
-        let space_atoms: Vec<Atom> = space.borrow().as_space().atom_iter().unwrap().cloned().collect();
+        let space_atoms = collect_atoms(space.borrow().as_space());
         assert_eq_no_order!(space_atoms, Vec::<Atom>::new());
     }
 
@@ -302,7 +309,7 @@ mod tests {
         let satom = Atom::gnd(space.clone());
         let res = AddAtomOp{}.execute(&mut vec![satom, expr!(("foo" "bar"))]).expect("No result returned");
         assert_eq!(res, vec![UNIT_ATOM]);
-        let space_atoms: Vec<Atom> = space.borrow().as_space().atom_iter().unwrap().cloned().collect();
+        let space_atoms = collect_atoms(space.borrow().as_space());
         assert_eq_no_order!(space_atoms, vec![expr!(("foo" "bar"))]);
     }
 

lib/src/space/grounding.rs

@@ -316,9 +316,6 @@ impl Space for GroundingSpace {
     fn atom_count(&self) -> Option<usize> {
         Some(self.iter().count())
     }
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        Some(Box::new(self.iter()))
-    }
     fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
         Ok(self.iter().for_each(|atom| v.accept(Cow::Borrowed(atom))))
     }

lib/src/space/mod.rs

@@ -198,11 +198,6 @@ pub trait Space: std::fmt::Debug + std::fmt::Display {
         None
     }
 
-    /// Returns an iterator over every atom in the space, or None if that is not possible
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        None
-    }
-
     /// Visit each atom of the space and call [SpaceVisitor::accept] method.
     /// This method is optional. Return `Err(())` if method is not implemented.
     /// `Cow<Atom>` is used to allow passing both references and values. First
@@ -345,9 +340,6 @@ impl Space for DynSpace {
     fn atom_count(&self) -> Option<usize> {
         self.0.borrow().atom_count()
     }
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        None
-    }
     fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
         self.0.borrow().visit(v)
     }
@@ -394,9 +386,6 @@ impl<T: Space> Space for &T {
     fn atom_count(&self) -> Option<usize> {
         T::atom_count(*self)
     }
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        T::atom_iter(*self)
-    }
     fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
         T::visit(*self, v)
     }

lib/src/space/module.rs

@@ -63,9 +63,6 @@ impl Space for ModuleSpace {
     fn atom_count(&self) -> Option<usize> {
         self.main.atom_count()
     }
-    fn atom_iter(&self) -> Option<Box<dyn Iterator<Item=&Atom> + '_>> {
-        self.main.atom_iter()
-    }
     fn visit(&self, v: &mut dyn SpaceVisitor) -> Result<(), ()> {
         self.main.visit(v)
     }