netdoc: fix behavior when using a fancy closure with PauseAt.

Previously, every call to peek() or next() would call self.pred().
But this would run us into trouble if we were using a closure that
had mutable state, since it would stop us from checking for things
like "the third occurrence of the foo token".

Now we store the value of self.pred(self.peek()).

tor-netdoc/src/util.rs

@@ -5,9 +5,15 @@ use std::iter::Peekable;
 ///
 /// Unlike std::iter::TakeWhile, it doesn't consume the first non-returned
 /// element.
+///
+/// We guarantee that the predicate is called no more than once for
+/// each item.
 pub struct PauseAt<'a, I: Iterator, F: FnMut(&I::Item) -> bool> {
     peek: &'a mut Peekable<I>,
     pred: F,
+    /// Memoized value of self.pred(self.peek()), so we never
+    /// calculate it more than once.
+    paused: Option<bool>,
 }
 
 impl<'a, I: Iterator, F: FnMut(&I::Item) -> bool> PauseAt<'a, I, F> {
@@ -15,7 +21,11 @@ impl<'a, I: Iterator, F: FnMut(&I::Item) -> bool> PauseAt<'a, I, F> {
     where
         F: FnMut(&I::Item) -> bool,
     {
-        PauseAt { peek, pred }
+        PauseAt {
+            peek,
+            pred,
+            paused: None,
+        }
     }
     /// Replace the predicate on a PauseAt, returning a new PauseAt.
     pub fn new_pred<F2>(self, pred: F2) -> PauseAt<'a, I, F2>
@@ -33,14 +43,26 @@ impl<'a, I: Iterator, F: FnMut(&I::Item) -> bool> PauseAt<'a, I, F> {
     pub fn peek(&mut self) -> Option<&I::Item> {
         // TODO: I wish it weren't necessary for this function to take
         // a mutable reference.
-        if let Some(nextval) = self.peek.peek() {
-            if (self.pred)(nextval) {
-                None
-            } else {
-                Some(nextval)
-            }
-        } else {
+        if self.check_paused() {
             None
+        } else {
+            self.peek.peek()
+        }
+    }
+    /// Helper: Return true if we will pause before the next element,
+    /// and false otherwise.  Store the value in self.paused, so that
+    /// we don't invoke self.pred more than once.
+    fn check_paused(&mut self) -> bool {
+        if let Some(p) = self.paused {
+            return p;
+        }
+        if let Some(nextval) = self.peek.peek() {
+            let p = (self.pred)(nextval);
+            self.paused = Some(p);
+            p
+        } else {
+            self.paused = Some(false);
+            false
         }
     }
 }
@@ -48,14 +70,11 @@ impl<'a, I: Iterator, F: FnMut(&I::Item) -> bool> PauseAt<'a, I, F> {
 impl<'a, I: Iterator, F: FnMut(&I::Item) -> bool> Iterator for PauseAt<'a, I, F> {
     type Item = I::Item;
     fn next(&mut self) -> Option<Self::Item> {
-        if let Some(nextval) = self.peek.peek() {
-            if (self.pred)(nextval) {
-                None
-            } else {
-                self.peek.next()
-            }
-        } else {
+        if self.check_paused() {
             None
+        } else {
+            self.paused = None;
+            self.peek.next()
         }
     }
 }
@@ -128,6 +147,27 @@ mod tests {
         assert_eq!(iter.next(), None);
     }
 
+    #[test]
+    fn test_parse_at_mutable() {
+        use super::PauseAt;
+        let mut count = 0;
+        let mut iter = (1..10).into_iter().peekable();
+        // pause at the third item, using mutable state in the closure.
+        let mut iter = PauseAt::from_peekable(&mut iter, |_| {
+            count += 1;
+            count == 4
+        });
+        assert_eq!(iter.peek(), Some(&1)); // we can do this multiple times,
+        assert_eq!(iter.peek(), Some(&1)); // but count isn't advanced.
+        assert_eq!(iter.next(), Some(1));
+
+        assert_eq!(iter.next(), Some(2));
+        assert_eq!(iter.peek(), Some(&3));
+        assert_eq!(iter.next(), Some(3));
+        assert_eq!(iter.peek(), None);
+        assert_eq!(iter.next(), None);
+    }
+
     #[test]
     fn test_str_offset() {
         use super::str_offset;