rust: propagate error of index_get_parents() properly
authorYuya Nishihara <yuya@tcha.org>
Mon, 29 Oct 2018 21:50:53 +0900
changeset 53628 443eb4bc41af00cafefcf235f15bad24b1ea56a1
parent 53627 54a60968f0aa7ee74fed464b832ab6d0bbf9af74
child 53629 09680349cc2d9363bcd9b6bc7d9f865d69c14911
push id1079
push usergszorc@mozilla.com
push dateMon, 10 Dec 2018 19:44:59 +0000
rust: propagate error of index_get_parents() properly Before, rustla_contains() would return 0 on error, and the exception would be cleared or noticed somewhere else. We need to propagate the error from AncestorsIterator up to the FFI surface.
rust/hg-core/src/ancestors.rs
rust/hg-direct-ffi/src/ancestors.rs
--- a/rust/hg-core/src/ancestors.rs
+++ b/rust/hg-core/src/ancestors.rs
@@ -72,29 +72,30 @@ impl<G: Graph> AncestorsIterator<G> {
         }
     }
 
     /// Consumes partially the iterator to tell if the given target
     /// revision
     /// is in the ancestors it emits.
     /// This is meant for iterators actually dedicated to that kind of
     /// purpose
-    pub fn contains(&mut self, target: Revision) -> bool {
+    pub fn contains(&mut self, target: Revision) -> Result<bool, GraphError> {
         if self.seen.contains(&target) && target != NULL_REVISION {
-            return true;
+            return Ok(true);
         }
-        for rev in self {
+        for item in self {
+            let rev = item?;
             if rev == target {
-                return true;
+                return Ok(true);
             }
             if rev < target {
-                return false;
+                return Ok(false);
             }
         }
-        false
+        Ok(false)
     }
 }
 
 /// Main implementation.
 ///
 /// The algorithm is the same as in `_lazyancestorsiter()` from `ancestors.py`
 /// with a few non crucial differences:
 ///
@@ -112,38 +113,38 @@ impl<G: Graph> AncestorsIterator<G> {
 ///
 /// - there's a good chance that invalid revisionss are fed from the start,
 ///   and `new()` doesn't swallow the error result.
 /// - this is probably what the Python implementation produces anyway, due
 ///   to filtering at each step, and Python code is currently the only
 ///   concrete caller we target, so we shouldn't need a finer error treatment
 ///   for the time being.
 impl<G: Graph> Iterator for AncestorsIterator<G> {
-    type Item = Revision;
+    type Item = Result<Revision, GraphError>;
 
-    fn next(&mut self) -> Option<Revision> {
+    fn next(&mut self) -> Option<Self::Item> {
         let current = match self.visit.peek() {
             None => {
                 return None;
             }
             Some(c) => *c,
         };
-        let (p1, p2) = self
-            .graph
-            .parents(current)
-            .unwrap_or((NULL_REVISION, NULL_REVISION));
+        let (p1, p2) = match self.graph.parents(current) {
+            Ok(ps) => ps,
+            Err(e) => return Some(Err(e)),
+        };
         if p1 < self.stoprev || self.seen.contains(&p1) {
             self.visit.pop();
         } else {
             *(self.visit.peek_mut().unwrap()) = p1;
             self.seen.insert(p1);
         };
 
         self.conditionally_push_rev(p2);
-        Some(current)
+        Some(Ok(current))
     }
 }
 
 #[cfg(test)]
 mod tests {
 
     use super::*;
 
--- a/rust/hg-direct-ffi/src/ancestors.rs
+++ b/rust/hg-direct-ffi/src/ancestors.rs
@@ -134,17 +134,21 @@ fn raw_drop<G: Graph>(raw_iter: *mut Anc
 pub extern "C" fn rustlazyancestors_next(raw: *mut AncestorsIterator<Index>) -> c_long {
     raw_next(raw)
 }
 
 /// Testable (for any Graph) version of rustlazayancestors_next
 #[inline]
 fn raw_next<G: Graph>(raw: *mut AncestorsIterator<G>) -> c_long {
     let as_ref = unsafe { &mut *raw };
-    as_ref.next().unwrap_or(NULL_REVISION) as c_long
+    let rev = match as_ref.next() {
+        Some(Ok(rev)) => rev,
+        Some(Err(_)) | None => NULL_REVISION,
+    };
+    rev as c_long
 }
 
 #[no_mangle]
 pub extern "C" fn rustlazyancestors_contains(
     raw: *mut AncestorsIterator<Index>,
     target: c_long,
 ) -> c_int {
     raw_contains(raw, target)
@@ -152,20 +156,20 @@ pub extern "C" fn rustlazyancestors_cont
 
 /// Testable (for any Graph) version of rustlazayancestors_next
 #[inline]
 fn raw_contains<G: Graph>(
     raw: *mut AncestorsIterator<G>,
     target: c_long,
 ) -> c_int {
     let as_ref = unsafe { &mut *raw };
-    if as_ref.contains(target as Revision) {
-        return 1;
+    match as_ref.contains(target as Revision) {
+        Ok(r) => r as c_int,
+        Err(_) => -1,
     }
-    0
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use std::thread;
 
     #[derive(Clone, Debug)]