Patchwork D11465: rust: Remove EntryState::Unknown

login
register
mail settings
Submitter phabricator
Date Sept. 20, 2021, 10:19 p.m.
Message ID <differential-rev-PHID-DREV-rfrkzrcz53aue537pefx-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/49779/
State Superseded
Headers show

Comments

phabricator - Sept. 20, 2021, 10:19 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This enum variant represented the `state == '?'` case, which was used
  to represent the absence of a dirstate entry/item (and therefore of that
  entry’s state).
  
  Now that previous refactors have removed this use in the Python/Rust
  FFI APIs, the remaining uses can be removed by replacing `EntryState`
  by `Option<EntryState>` where appropriate, using `None` to represent
  the absence of an entry.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS




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

Patch

diff --git a/rust/hg-core/src/dirstate_tree/status.rs b/rust/hg-core/src/dirstate_tree/status.rs
--- a/rust/hg-core/src/dirstate_tree/status.rs
+++ b/rust/hg-core/src/dirstate_tree/status.rs
@@ -394,9 +394,6 @@ 
                             .push(hg_path.detach_from_tree()),
                         EntryState::Normal => self
                             .handle_normal_file(&dirstate_node, fs_metadata)?,
-                        // This variant is not used in DirstateMap
-                        // nodes
-                        EntryState::Unknown => unreachable!(),
                     }
                 } else {
                     // `node.entry.is_none()` indicates a "directory"
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
@@ -593,12 +593,13 @@ 
     fn add_or_remove_file(
         &mut self,
         path: &HgPath,
-        old_state: EntryState,
+        old_state: Option<EntryState>,
         new_entry: DirstateEntry,
     ) -> Result<(), DirstateV2ParseError> {
-        let had_entry = old_state != EntryState::Unknown;
+        let had_entry = old_state.is_some();
+        let was_tracked = old_state.map_or(false, |s| s.is_tracked());
         let tracked_count_increment =
-            match (old_state.is_tracked(), new_entry.state().is_tracked()) {
+            match (was_tracked, new_entry.state().is_tracked()) {
                 (false, true) => 1,
                 (true, false) => -1,
                 _ => 0,
@@ -808,10 +809,7 @@ 
         let mode = entry.mode();
         let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
 
-        let old_state = match self.get(filename)? {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
+        let old_state = self.get(filename)?.map(|e| e.state());
 
         Ok(self.add_or_remove_file(filename, old_state, entry)?)
     }
@@ -822,10 +820,7 @@ 
         in_merge: bool,
     ) -> Result<(), DirstateError> {
         let old_entry_opt = self.get(filename)?;
-        let old_state = match old_entry_opt {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
+        let old_state = old_entry_opt.map(|e| e.state());
         let mut size = 0;
         if in_merge {
             // XXX we should not be able to have 'm' state and 'FROM_P2' if not
@@ -852,10 +847,9 @@ 
     }
 
     fn drop_file(&mut self, filename: &HgPath) -> Result<bool, DirstateError> {
-        let old_state = match self.get(filename)? {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
+        let was_tracked = self
+            .get(filename)?
+            .map_or(false, |e| e.state().is_tracked());
         struct Dropped {
             was_tracked: bool,
             had_entry: bool,
@@ -955,7 +949,7 @@ 
             }
             Ok(dropped.had_entry)
         } else {
-            debug_assert!(!old_state.is_tracked());
+            debug_assert!(!was_tracked);
             Ok(false)
         }
     }
diff --git a/rust/hg-core/src/dirstate/status.rs b/rust/hg-core/src/dirstate/status.rs
--- a/rust/hg-core/src/dirstate/status.rs
+++ b/rust/hg-core/src/dirstate/status.rs
@@ -205,7 +205,6 @@ 
         EntryState::Merged => Dispatch::Modified,
         EntryState::Added => Dispatch::Added,
         EntryState::Removed => Dispatch::Removed,
-        EntryState::Unknown => Dispatch::Unknown,
     }
 }
 
@@ -218,8 +217,6 @@ 
         }
         // File was removed, everything is normal
         EntryState::Removed => Dispatch::Removed,
-        // File is unknown to Mercurial, everything is normal
-        EntryState::Unknown => Dispatch::Unknown,
     }
 }
 
diff --git a/rust/hg-core/src/dirstate/entry.rs b/rust/hg-core/src/dirstate/entry.rs
--- a/rust/hg-core/src/dirstate/entry.rs
+++ b/rust/hg-core/src/dirstate/entry.rs
@@ -7,7 +7,6 @@ 
     Added,
     Removed,
     Merged,
-    Unknown,
 }
 
 /// The C implementation uses all signed types. This will be an issue
@@ -157,7 +156,7 @@ 
         use EntryState::*;
         match self {
             Normal | Added | Merged => true,
-            Removed | Unknown => false,
+            Removed => false,
         }
     }
 }
@@ -171,7 +170,6 @@ 
             b'a' => Ok(EntryState::Added),
             b'r' => Ok(EntryState::Removed),
             b'm' => Ok(EntryState::Merged),
-            b'?' => Ok(EntryState::Unknown),
             _ => Err(HgError::CorruptedRepository(format!(
                 "Incorrect dirstate entry state {}",
                 value
@@ -187,7 +185,6 @@ 
             EntryState::Added => b'a',
             EntryState::Removed => b'r',
             EntryState::Merged => b'm',
-            EntryState::Unknown => b'?',
         }
     }
 }
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -111,17 +111,13 @@ 
         let mode = entry.mode();
         let entry = DirstateEntry::from_v1_data(state, mode, size, mtime);
 
-        let old_state = match self.get(filename) {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
-        if old_state == EntryState::Unknown || old_state == EntryState::Removed
-        {
+        let old_state = self.get(filename).map(|e| e.state());
+        if old_state.is_none() || old_state == Some(EntryState::Removed) {
             if let Some(ref mut dirs) = self.dirs {
                 dirs.add_path(filename)?;
             }
         }
-        if old_state == EntryState::Unknown {
+        if old_state.is_none() {
             if let Some(ref mut all_dirs) = self.all_dirs {
                 all_dirs.add_path(filename)?;
             }
@@ -153,10 +149,7 @@ 
         in_merge: bool,
     ) -> Result<(), DirstateError> {
         let old_entry_opt = self.get(filename);
-        let old_state = match old_entry_opt {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
+        let old_state = old_entry_opt.map(|e| e.state());
         let mut size = 0;
         if in_merge {
             // XXX we should not be able to have 'm' state and 'FROM_P2' if not
@@ -178,13 +171,12 @@ 
                 }
             }
         }
-        if old_state != EntryState::Unknown && old_state != EntryState::Removed
-        {
+        if old_state.is_some() && old_state != Some(EntryState::Removed) {
             if let Some(ref mut dirs) = self.dirs {
                 dirs.delete_path(filename)?;
             }
         }
-        if old_state == EntryState::Unknown {
+        if old_state.is_none() {
             if let Some(ref mut all_dirs) = self.all_dirs {
                 all_dirs.add_path(filename)?;
             }
@@ -207,14 +199,11 @@ 
         &mut self,
         filename: &HgPath,
     ) -> Result<bool, DirstateError> {
-        let old_state = match self.get(filename) {
-            Some(e) => e.state(),
-            None => EntryState::Unknown,
-        };
+        let old_state = self.get(filename).map(|e| e.state());
         let exists = self.state_map.remove(filename).is_some();
 
         if exists {
-            if old_state != EntryState::Removed {
+            if old_state != Some(EntryState::Removed) {
                 if let Some(ref mut dirs) = self.dirs {
                     dirs.delete_path(filename)?;
                 }