Patchwork D12530: rust-dirstatemap: use `get_node_mut` instead or `get_or_insert_node`

login
register
mail settings
Submitter phabricator
Date April 12, 2022, 4:05 p.m.
Message ID <differential-rev-PHID-DREV-fprwy4w6a4ktamsswif4-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50878/
State New
Headers show

Comments

phabricator - April 12, 2022, 4:05 p.m.
Alphare created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This (along with the docstring), makes it more obvious that we're not expecting
  to insert a node here. This is less prone to bugs in later refactorings.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D12530

AFFECTED FILES
  rust/hg-core/src/dirstate_tree/dirstate_map.rs

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/hg-core/src/dirstate_tree/dirstate_map.rs b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs
@@ -733,25 +733,45 @@ 
         Ok(new)
     }
 
-    /// It is the responsibility of the caller to know that there was an entry
-    /// there before. Does not handle the removal of copy source
+    /// Set a node as untracked in the dirstate.
+    ///
+    /// It is the responsibility of the caller to remove the copy source and/or
+    /// the entry itself if appropriate.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the node does not exist.
     fn set_untracked(
         &mut self,
         filename: &HgPath,
         old_entry: DirstateEntry,
     ) -> Result<(), DirstateV2ParseError> {
-        let node = self.get_or_insert_node(filename, |ancestor| {
-            ancestor.tracked_descendants_count = ancestor
-                .tracked_descendants_count
-                .checked_sub(1)
-                .expect("tracked_descendants_count should be >= 0");
-        })?;
+        let node = DirstateMap::get_node_mut(
+            self.on_disk,
+            &mut self.unreachable_bytes,
+            &mut self.root,
+            filename,
+            |ancestor| {
+                ancestor.tracked_descendants_count = ancestor
+                    .tracked_descendants_count
+                    .checked_sub(1)
+                    .expect("tracked_descendants_count should be >= 0");
+            },
+        )?
+        .expect("node should exist");
         let mut new_entry = old_entry.clone();
         new_entry.set_untracked();
         node.data = NodeData::Entry(new_entry);
         Ok(())
     }
 
+    /// Set a node as clean in the dirstate.
+    ///
+    /// It is the responsibility of the caller to remove the copy source.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the node does not exist.
     fn set_clean(
         &mut self,
         filename: &HgPath,
@@ -760,22 +780,41 @@ 
         size: u32,
         mtime: TruncatedTimestamp,
     ) -> Result<(), DirstateError> {
-        let node = self.get_or_insert_node(filename, |ancestor| {
-            if !old_entry.tracked() {
-                ancestor.tracked_descendants_count += 1;
-            }
-        })?;
+        let node = DirstateMap::get_node_mut(
+            self.on_disk,
+            &mut self.unreachable_bytes,
+            &mut self.root,
+            filename,
+            |ancestor| {
+                if !old_entry.tracked() {
+                    ancestor.tracked_descendants_count += 1;
+                }
+            },
+        )?
+        .expect("node should exist");
         let mut new_entry = old_entry.clone();
         new_entry.set_clean(mode, size, mtime);
         node.data = NodeData::Entry(new_entry);
         Ok(())
     }
 
+    /// Set a node as possibly dirty in the dirstate.
+    ///
+    /// # Panics
+    ///
+    /// Panics if the node does not exist.
     fn set_possibly_dirty(
         &mut self,
         filename: &HgPath,
     ) -> Result<(), DirstateError> {
-        let node = self.get_or_insert_node(filename, |_ancestor| {})?;
+        let node = DirstateMap::get_node_mut(
+            self.on_disk,
+            &mut self.unreachable_bytes,
+            &mut self.root,
+            filename,
+            |_ancestor| {},
+        )?
+        .expect("node should exist");
         let entry = node.data.as_entry_mut().expect("entry should exist");
         entry.set_possibly_dirty();
         node.data = NodeData::Entry(*entry);