cleanup: simplify some uses of Option, mostly by using ?

lib/src/dag_walk.rs

@@ -33,8 +33,8 @@ where
     type Item = T;
 
     fn next(&mut self) -> Option<Self::Item> {
-        while !self.work.is_empty() {
+        loop {
-            let c = self.work.pop().unwrap();
+            let c = self.work.pop()?;
             let id = (self.id_fn)(&c);
             if self.visited.contains(&id) {
                 continue;
@@ -45,7 +45,6 @@ where
             self.visited.insert(id);
             return Some(c);
         }
-        None
     }
 }
 

lib/src/revset.rs

@@ -938,7 +938,8 @@ impl<'repo> Iterator for ChildrenRevsetIterator<'_, 'repo> {
     type Item = IndexEntry<'repo>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        while let Some(candidate) = self.candidate_iter.next() {
+        loop {
+            let candidate = self.candidate_iter.next()?;
             if candidate
                 .parent_positions()
                 .iter()
@@ -947,7 +948,6 @@ impl<'repo> Iterator for ChildrenRevsetIterator<'_, 'repo> {
                 return Some(candidate);
             }
         }
-        None
     }
 }
 

lib/src/revset_graph_iterator.rs

@@ -294,16 +294,13 @@ impl<'revset, 'repo> Iterator for RevsetGraphIterator<'revset, 'repo> {
     type Item = (IndexEntry<'repo>, Vec<RevsetGraphEdge>);
 
     fn next(&mut self) -> Option<Self::Item> {
-        if let Some(index_entry) = self.next_index_entry() {
+        let index_entry = self.next_index_entry()?;
-            let mut edges = self.edges_from_internal_commit(&index_entry);
+        let mut edges = self.edges_from_internal_commit(&index_entry);
-            if self.skip_transitive_edges {
+        if self.skip_transitive_edges {
-                edges = self.remove_transitive_edges(edges);
+            edges = self.remove_transitive_edges(edges);
-            }
-            let mut edges: Vec<_> = edges.into_iter().collect();
-            edges.sort_by(|edge1, edge2| edge2.target.cmp(&edge1.target));
-            Some((index_entry, edges))
-        } else {
-            None
         }
+        let mut edges: Vec<_> = edges.into_iter().collect();
+        edges.sort_by(|edge1, edge2| edge2.target.cmp(&edge1.target));
+        Some((index_entry, edges))
     }
 }

lib/src/stacked_table.rs

@@ -51,15 +51,13 @@ pub trait TableSegment {
 
     fn get_value<'a>(&'a self, key: &[u8]) -> Option<&'a [u8]> {
         if let Some(value) = self.segment_get_value(key) {
-            Some(value)
+            return Some(value);
-        } else if let Some(parent_file) = self.segment_parent_file() {
-            let parent_file: &ReadonlyTable = parent_file.as_ref();
-            // The parent ReadonlyIndex outlives the child
-            let parent_file: &'a ReadonlyTable = unsafe { std::mem::transmute(parent_file) };
-            parent_file.get_value(key)
-        } else {
-            None
         }
+        let parent_file = self.segment_parent_file()?;
+        let parent_file: &ReadonlyTable = parent_file.as_ref();
+        // The parent ReadonlyIndex outlives the child
+        let parent_file: &'a ReadonlyTable = unsafe { std::mem::transmute(parent_file) };
+        parent_file.get_value(key)
     }
 }
 

lib/src/tree.rs

@@ -248,24 +248,21 @@ impl Iterator for TreeEntriesIterator<'_> {
                 }
                 self.subdir_iterator = None;
             }
-            if let Some(entry) = self.entry_iterator.next() {
+            let entry = self.entry_iterator.next()?;
-                match entry.value() {
+            match entry.value() {
-                    TreeValue::Tree(id) => {
+                TreeValue::Tree(id) => {
-                        let subtree = self.tree.known_sub_tree(entry.name(), id);
+                    let subtree = self.tree.known_sub_tree(entry.name(), id);
-                        self.subdir_iterator =
+                    self.subdir_iterator =
-                            Some(Box::new(TreeEntriesIterator::new(subtree, self.matcher)));
+                        Some(Box::new(TreeEntriesIterator::new(subtree, self.matcher)));
+                }
+                other => {
+                    let path = self.tree.dir().join(entry.name());
+                    if !self.matcher.matches(&path) {
+                        continue;
                     }
-                    other => {
+                    return Some((path, other.clone()));
-                        let path = self.tree.dir().join(entry.name());
+                }
-                        if !self.matcher.matches(&path) {
+            };
-                            continue;
-                        }
-                        return Some((path, other.clone()));
-                    }
-                };
-            } else {
-                return None;
-            }
         }
     }
 }
@@ -444,63 +441,58 @@ impl Iterator for TreeDiffIterator<'_> {
             }
 
             // Note: whenever we say "file" below, it may also be a symlink or a conflict.
-            if let Some((name, before, after)) = self.entry_iterator.next() {
+            let (name, before, after) = self.entry_iterator.next()?;
-                let tree_before = matches!(before, Some(TreeValue::Tree(_)));
+            let tree_before = matches!(before, Some(TreeValue::Tree(_)));
-                let tree_after = matches!(after, Some(TreeValue::Tree(_)));
+            let tree_after = matches!(after, Some(TreeValue::Tree(_)));
-                if tree_before || tree_after {
+            if tree_before || tree_after {
-                    let subdir = &name;
+                let subdir = &name;
-                    let subdir_path = self.dir.join(subdir);
+                let subdir_path = self.dir.join(subdir);
-                    let before_tree = match before {
+                let before_tree = match before {
-                        Some(TreeValue::Tree(id_before)) => {
+                    Some(TreeValue::Tree(id_before)) => {
-                            self.tree1.known_sub_tree(subdir, id_before)
+                        self.tree1.known_sub_tree(subdir, id_before)
+                    }
+                    _ => Tree::null(self.tree1.store().clone(), subdir_path.clone()),
+                };
+                let after_tree = match after {
+                    Some(TreeValue::Tree(id_after)) => self.tree2.known_sub_tree(subdir, id_after),
+                    _ => Tree::null(self.tree2.store().clone(), subdir_path.clone()),
+                };
+                self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
+                    subdir_path,
+                    before_tree,
+                    after_tree,
+                    self.matcher,
+                )));
+            }
+            let file_path = self.dir.join(&name);
+            if self.matcher.matches(&file_path) {
+                if !tree_before && tree_after {
+                    if let Some(file_before) = before {
+                        return Some((file_path, Diff::Removed(file_before.clone())));
+                    }
+                } else if tree_before && !tree_after {
+                    if let Some(file_after) = after {
+                        self.added_file = Some((file_path, file_after.clone()));
+                    }
+                } else if !tree_before && !tree_after {
+                    match (before, after) {
+                        (Some(file_before), Some(file_after)) => {
+                            return Some((
+                                file_path,
+                                Diff::Modified(file_before.clone(), file_after.clone()),
+                            ));
                         }
-                        _ => Tree::null(self.tree1.store().clone(), subdir_path.clone()),
+                        (None, Some(file_after)) => {
-                    };
+                            return Some((file_path, Diff::Added(file_after.clone())));
-                    let after_tree = match after {
-                        Some(TreeValue::Tree(id_after)) => {
-                            self.tree2.known_sub_tree(subdir, id_after)
                         }
-                        _ => Tree::null(self.tree2.store().clone(), subdir_path.clone()),
+                        (Some(file_before), None) => {
-                    };
-                    self.subdir_iterator = Some(Box::new(TreeDiffIterator::new(
-                        subdir_path,
-                        before_tree,
-                        after_tree,
-                        self.matcher,
-                    )));
-                }
-                let file_path = self.dir.join(&name);
-                if self.matcher.matches(&file_path) {
-                    if !tree_before && tree_after {
-                        if let Some(file_before) = before {
                             return Some((file_path, Diff::Removed(file_before.clone())));
                         }
-                    } else if tree_before && !tree_after {
+                        (None, None) => {
-                        if let Some(file_after) = after {
+                            panic!("unexpected diff")
-                            self.added_file = Some((file_path, file_after.clone()));
-                        }
-                    } else if !tree_before && !tree_after {
-                        match (before, after) {
-                            (Some(file_before), Some(file_after)) => {
-                                return Some((
-                                    file_path,
-                                    Diff::Modified(file_before.clone(), file_after.clone()),
-                                ));
-                            }
-                            (None, Some(file_after)) => {
-                                return Some((file_path, Diff::Added(file_after.clone())));
-                            }
-                            (Some(file_before), None) => {
-                                return Some((file_path, Diff::Removed(file_before.clone())));
-                            }
-                            (None, None) => {
-                                panic!("unexpected diff")
-                            }
                         }
                     }
                 }
-            } else {
-                return None;
             }
         }
     }

lib/src/workspace.rs

@@ -251,10 +251,6 @@ fn find_jj_dir(mut workspace_root: &Path) -> Option<PathBuf> {
         if jj_path.is_dir() {
             return Some(jj_path);
         }
-        if let Some(wc_dir_parent) = workspace_root.parent() {
+        workspace_root = workspace_root.parent()?;
-            workspace_root = wc_dir_parent;
-        } else {
-            return None;
-        }
     }
 }