rust: use the new `UncheckedRevision` everywhere applicable

This step converts all revisions that shouldn't be considered "valid" in any
context to `UncheckedRevison`, allowing `Revision` to be changed for a
stronger type in a later changeset.

Note that the conversion from unchecked to checked is manual and requires
at least some thought from the programmer, although directly using `Revision`
is still possible. A later changeset will make this mistake harder to make.

rust/hg-core/src/operations/cat.rs

@@ -84,10 +84,10 b" pub fn cat<'a>("
     mut files: Vec<&'a HgPath>,
 ) -> Result<CatOutput<'a>, RevlogError> {
     let rev = crate::revset::resolve_single(revset, repo)?;
-    let manifest = repo.manifest_for_rev(rev)?;
+    let manifest = repo.manifest_for_rev(rev.into())?;
     let node = *repo
         .changelog()?
-        .node_from_rev(rev)
+        .node_from_rev(rev.into())
         .expect("should succeed when repo.manifest did");
     let mut results: Vec<(&'a HgPath, Vec<u8>)> = vec![];
     let mut found_any = false;

rust/hg-core/src/operations/debugdata.rs

@@ -33,6 +33,6 b' pub fn debug_data('
         Revlog::open(&repo.store_vfs(), index_file, None, use_nodemap)?;
     let rev =
         crate::revset::resolve_rev_number_or_hex_prefix(revset, &revlog)?;
-    let data = revlog.get_rev_data(rev)?;
+    let data = revlog.get_rev_data_for_checked_rev(rev)?;
     Ok(data.into_owned())
 }

rust/hg-core/src/operations/list_tracked_files.rs

@@ -21,7 +21,7 b' pub fn list_rev_tracked_files('
 ) -> Result<FilesForRev, RevlogError> {
     let rev = crate::revset::resolve_single(revset, repo)?;
     Ok(FilesForRev {
-        manifest: repo.manifest_for_rev(rev)?,
+        manifest: repo.manifest_for_rev(rev.into())?,
         narrow_matcher,
     })
 }

rust/hg-core/src/repo.rs

@@ -15,8 +15,8 b' use crate::utils::files::get_path_from_b'
 use crate::utils::hg_path::HgPath;
 use crate::utils::SliceExt;
 use crate::vfs::{is_dir, is_file, Vfs};
-use crate::{requirements, NodePrefix};
-use crate::{DirstateError, Revision};
+use crate::DirstateError;
+use crate::{requirements, NodePrefix, UncheckedRevision};
 use std::cell::{Ref, RefCell, RefMut};
 use std::collections::HashSet;
 use std::io::Seek;
@@ -562,7 +562,7 b' impl Repo {'
     /// Returns the manifest of the *changeset* with the given revision number
     pub fn manifest_for_rev(
         &self,
-        revision: Revision,
+        revision: UncheckedRevision,
     ) -> Result<Manifest, RevlogError> {
         self.manifestlog()?.data_for_node(
             self.changelog()?

rust/hg-core/src/revlog/changelog.rs

@@ -4,6 +4,7 b' use crate::revlog::{Node, NodePrefix};'
 use crate::revlog::{Revlog, RevlogEntry, RevlogError};
 use crate::utils::hg_path::HgPath;
 use crate::vfs::Vfs;
+use crate::UncheckedRevision;
 use itertools::Itertools;
 use std::ascii::escape_default;
 use std::borrow::Cow;
@@ -29,15 +30,24 b' impl Changelog {'
         node: NodePrefix,
     ) -> Result<ChangelogRevisionData, RevlogError> {
         let rev = self.revlog.rev_from_node(node)?;
-        self.data_for_rev(rev)
+        self.entry_for_checked_rev(rev)?.data()
     }
 
     /// Return the [`ChangelogEntry`] for the given revision number.
     pub fn entry_for_rev(
         &self,
+        rev: UncheckedRevision,
+    ) -> Result<ChangelogEntry, RevlogError> {
+        let revlog_entry = self.revlog.get_entry(rev)?;
+        Ok(ChangelogEntry { revlog_entry })
+    }
+
+    /// Same as [`Self::entry_for_rev`] for checked revisions.
+    fn entry_for_checked_rev(
+        &self,
         rev: Revision,
     ) -> Result<ChangelogEntry, RevlogError> {
-        let revlog_entry = self.revlog.get_entry(rev)?;
+        let revlog_entry = self.revlog.get_entry_for_checked_rev(rev)?;
         Ok(ChangelogEntry { revlog_entry })
     }
 
@@ -49,12 +59,12 b' impl Changelog {'
     /// [entry_for_rev](`Self::entry_for_rev`) and doing everything from there.
     pub fn data_for_rev(
         &self,
-        rev: Revision,
+        rev: UncheckedRevision,
     ) -> Result<ChangelogRevisionData, RevlogError> {
         self.entry_for_rev(rev)?.data()
     }
 
-    pub fn node_from_rev(&self, rev: Revision) -> Option<&Node> {
+    pub fn node_from_rev(&self, rev: UncheckedRevision) -> Option<&Node> {
         self.revlog.node_from_rev(rev)
     }
 
@@ -330,12 +340,12 b' message",'
 
         let changelog = Changelog { revlog };
         assert_eq!(
-            changelog.data_for_rev(NULL_REVISION)?,
+            changelog.data_for_rev(NULL_REVISION.into())?,
             ChangelogRevisionData::null()
         );
         // same with the intermediate entry object
         assert_eq!(
-            changelog.entry_for_rev(NULL_REVISION)?.data()?,
+            changelog.entry_for_rev(NULL_REVISION.into())?.data()?,
             ChangelogRevisionData::null()
         );
         Ok(())

rust/hg-core/src/revlog/filelog.rs

@@ -1,4 +1,5 b''
 use crate::errors::HgError;
+use crate::exit_codes;
 use crate::repo::Repo;
 use crate::revlog::path_encode::path_encode;
 use crate::revlog::NodePrefix;
@@ -8,6 +9,7 b' use crate::revlog::{Revlog, RevlogError}'
 use crate::utils::files::get_path_from_bytes;
 use crate::utils::hg_path::HgPath;
 use crate::utils::SliceExt;
+use crate::UncheckedRevision;
 use std::path::PathBuf;
 
 /// A specialized `Revlog` to work with file data logs.
@@ -39,14 +41,14 b' impl Filelog {'
         file_node: impl Into<NodePrefix>,
     ) -> Result<FilelogRevisionData, RevlogError> {
         let file_rev = self.revlog.rev_from_node(file_node.into())?;
-        self.data_for_rev(file_rev)
+        self.data_for_rev(file_rev.into())
     }
 
     /// The given revision is that of the file as found in a filelog, not of a
     /// changeset.
     pub fn data_for_rev(
         &self,
-        file_rev: Revision,
+        file_rev: UncheckedRevision,
     ) -> Result<FilelogRevisionData, RevlogError> {
         let data: Vec<u8> = self.revlog.get_rev_data(file_rev)?.into_owned();
         Ok(FilelogRevisionData(data))
@@ -59,16 +61,25 b' impl Filelog {'
         file_node: impl Into<NodePrefix>,
     ) -> Result<FilelogEntry, RevlogError> {
         let file_rev = self.revlog.rev_from_node(file_node.into())?;
-        self.entry_for_rev(file_rev)
+        self.entry_for_checked_rev(file_rev)
     }
 
     /// The given revision is that of the file as found in a filelog, not of a
     /// changeset.
     pub fn entry_for_rev(
         &self,
+        file_rev: UncheckedRevision,
+    ) -> Result<FilelogEntry, RevlogError> {
+        Ok(FilelogEntry(self.revlog.get_entry(file_rev)?))
+    }
+
+    fn entry_for_checked_rev(
+        &self,
         file_rev: Revision,
     ) -> Result<FilelogEntry, RevlogError> {
-        Ok(FilelogEntry(self.revlog.get_entry(file_rev)?))
+        Ok(FilelogEntry(
+            self.revlog.get_entry_for_checked_rev(file_rev)?,
+        ))
     }
 }
 
@@ -165,7 +176,19 b" impl FilelogEntry<'_> {"
     }
 
     pub fn data(&self) -> Result<FilelogRevisionData, HgError> {
-        Ok(FilelogRevisionData(self.0.data()?.into_owned()))
+        let data = self.0.data();
+        match data {
+            Ok(data) => Ok(FilelogRevisionData(data.into_owned())),
+            // Errors other than `HgError` should not happen at this point
+            Err(e) => match e {
+                RevlogError::Other(hg_error) => Err(hg_error),
+                revlog_error => Err(HgError::abort(
+                    revlog_error.to_string(),
+                    exit_codes::ABORT,
+                    None,
+                )),
+            },
+        }
     }
 }
 

rust/hg-core/src/revlog/index.rs

@@ -1,3 +1,4 b''
+use std::fmt::Debug;
 use std::ops::Deref;
 
 use byteorder::{BigEndian, ByteOrder};
@@ -5,6 +6,7 b' use byteorder::{BigEndian, ByteOrder};'
 use crate::errors::HgError;
 use crate::revlog::node::Node;
 use crate::revlog::{Revision, NULL_REVISION};
+use crate::UncheckedRevision;
 
 pub const INDEX_ENTRY_SIZE: usize = 64;
 
@@ -86,6 +88,15 b' pub struct Index {'
     uses_generaldelta: bool,
 }
 
+impl Debug for Index {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Index")
+            .field("offsets", &self.offsets)
+            .field("uses_generaldelta", &self.uses_generaldelta)
+            .finish()
+    }
+}
+
 impl Index {
     /// Create an index from bytes.
     /// Calculate the start of each entry when is_inline is true.
@@ -175,36 +186,32 b' impl Index {'
         if rev == NULL_REVISION {
             return None;
         }
-        if let Some(offsets) = &self.offsets {
+        Some(if let Some(offsets) = &self.offsets {
             self.get_entry_inline(rev, offsets)
         } else {
             self.get_entry_separated(rev)
-        }
+        })
     }
 
     fn get_entry_inline(
         &self,
         rev: Revision,
         offsets: &[usize],
-    ) -> Option<IndexEntry> {
-        let start = *offsets.get(rev as usize)?;
-        let end = start.checked_add(INDEX_ENTRY_SIZE)?;
+    ) -> IndexEntry {
+        let start = offsets[rev as usize];
+        let end = start + INDEX_ENTRY_SIZE;
         let bytes = &self.bytes[start..end];
 
         // See IndexEntry for an explanation of this override.
         let offset_override = Some(end);
 
-        Some(IndexEntry {
+        IndexEntry {
             bytes,
             offset_override,
-        })
+        }
     }
 
-    fn get_entry_separated(&self, rev: Revision) -> Option<IndexEntry> {
-        let max_rev = self.bytes.len() / INDEX_ENTRY_SIZE;
-        if rev as usize >= max_rev {
-            return None;
-        }
+    fn get_entry_separated(&self, rev: Revision) -> IndexEntry {
         let start = rev as usize * INDEX_ENTRY_SIZE;
         let end = start + INDEX_ENTRY_SIZE;
         let bytes = &self.bytes[start..end];
@@ -213,10 +220,10 b' impl Index {'
         // for the index's metadata (saving space because it is always 0)
         let offset_override = if rev == 0 { Some(0) } else { None };
 
-        Some(IndexEntry {
+        IndexEntry {
             bytes,
             offset_override,
-        })
+        }
     }
 }
 
@@ -273,23 +280,23 b" impl<'a> IndexEntry<'a> {"
     }
 
     /// Return the revision upon which the data has been derived.
-    pub fn base_revision_or_base_of_delta_chain(&self) -> Revision {
+    pub fn base_revision_or_base_of_delta_chain(&self) -> UncheckedRevision {
         // TODO Maybe return an Option when base_revision == rev?
         //      Requires to add rev to IndexEntry
 
-        BigEndian::read_i32(&self.bytes[16..])
+        BigEndian::read_i32(&self.bytes[16..]).into()
     }
 
-    pub fn link_revision(&self) -> Revision {
-        BigEndian::read_i32(&self.bytes[20..])
+    pub fn link_revision(&self) -> UncheckedRevision {
+        BigEndian::read_i32(&self.bytes[20..]).into()
     }
 
-    pub fn p1(&self) -> Revision {
-        BigEndian::read_i32(&self.bytes[24..])
+    pub fn p1(&self) -> UncheckedRevision {
+        BigEndian::read_i32(&self.bytes[24..]).into()
     }
 
-    pub fn p2(&self) -> Revision {
-        BigEndian::read_i32(&self.bytes[28..])
+    pub fn p2(&self) -> UncheckedRevision {
+        BigEndian::read_i32(&self.bytes[28..]).into()
     }
 
     /// Return the hash of revision's full text.
@@ -547,7 +554,7 b' mod tests {'
             offset_override: None,
         };
 
-        assert_eq!(entry.base_revision_or_base_of_delta_chain(), 1)
+        assert_eq!(entry.base_revision_or_base_of_delta_chain(), 1.into())
     }
 
     #[test]
@@ -559,7 +566,7 b' mod tests {'
             offset_override: None,
         };
 
-        assert_eq!(entry.link_revision(), 123);
+        assert_eq!(entry.link_revision(), 123.into());
     }
 
     #[test]
@@ -571,7 +578,7 b' mod tests {'
             offset_override: None,
         };
 
-        assert_eq!(entry.p1(), 123);
+        assert_eq!(entry.p1(), 123.into());
     }
 
     #[test]
@@ -583,7 +590,7 b' mod tests {'
             offset_override: None,
         };
 
-        assert_eq!(entry.p2(), 123);
+        assert_eq!(entry.p2(), 123.into());
     }
 
     #[test]

rust/hg-core/src/revlog/manifest.rs

@@ -1,10 +1,10 b''
 use crate::errors::HgError;
-use crate::revlog::Revision;
 use crate::revlog::{Node, NodePrefix};
 use crate::revlog::{Revlog, RevlogError};
 use crate::utils::hg_path::HgPath;
 use crate::utils::SliceExt;
 use crate::vfs::Vfs;
+use crate::{Revision, UncheckedRevision};
 
 /// A specialized `Revlog` to work with `manifest` data format.
 pub struct Manifestlog {
@@ -32,7 +32,7 b' impl Manifestlog {'
         node: NodePrefix,
     ) -> Result<Manifest, RevlogError> {
         let rev = self.revlog.rev_from_node(node)?;
-        self.data_for_rev(rev)
+        self.data_for_checked_rev(rev)
     }
 
     /// Return the `Manifest` of a given revision number.
@@ -43,9 +43,18 b' impl Manifestlog {'
     /// See also `Repo::manifest_for_rev`
     pub fn data_for_rev(
         &self,
+        rev: UncheckedRevision,
+    ) -> Result<Manifest, RevlogError> {
+        let bytes = self.revlog.get_rev_data(rev)?.into_owned();
+        Ok(Manifest { bytes })
+    }
+
+    pub fn data_for_checked_rev(
+        &self,
         rev: Revision,
     ) -> Result<Manifest, RevlogError> {
-        let bytes = self.revlog.get_rev_data(rev)?.into_owned();
+        let bytes =
+            self.revlog.get_rev_data_for_checked_rev(rev)?.into_owned();
         Ok(Manifest { bytes })
     }
 }

rust/hg-core/src/revlog/mod.rs

@@ -47,7 +47,24 b' pub type Revision = i32;'
 ///
 /// As noted in revlog.c, revision numbers are actually encoded in
 /// 4 bytes, and are liberally converted to ints, whence the i32
-pub type UncheckedRevision = i32;
+#[derive(
+    Debug,
+    derive_more::Display,
+    Clone,
+    Copy,
+    Hash,
+    PartialEq,
+    Eq,
+    PartialOrd,
+    Ord,
+)]
+pub struct UncheckedRevision(i32);
+
+impl From<Revision> for UncheckedRevision {
+    fn from(value: Revision) -> Self {
+        Self(value)
+    }
+}
 
 /// Marker expressing the absence of a parent
 ///
@@ -60,7 +77,8 b' pub const NULL_REVISION: Revision = -1;'
 /// This is also equal to `i32::max_value()`, but it's better to spell
 /// it out explicitely, same as in `mercurial.node`
 #[allow(clippy::unreadable_literal)]
-pub const WORKING_DIRECTORY_REVISION: Revision = 0x7fffffff;
+pub const WORKING_DIRECTORY_REVISION: UncheckedRevision =
+    UncheckedRevision(0x7fffffff);
 
 pub const WORKING_DIRECTORY_HEX: &str =
     "ffffffffffffffffffffffffffffffffffffffff";
@@ -90,14 +108,14 b' pub trait RevlogIndex {'
         self.len() == 0
     }
 
-    /// Return a reference to the Node or `None` if rev is out of bounds
-    ///
-    /// `NULL_REVISION` is not considered to be out of bounds.
+    /// Return a reference to the Node or `None` for `NULL_REVISION`
     fn node(&self, rev: Revision) -> Option<&Node>;
 
     /// Return a [`Revision`] if `rev` is a valid revision number for this
     /// index
     fn check_revision(&self, rev: UncheckedRevision) -> Option<Revision> {
+        let rev = rev.0;
+
         if rev == NULL_REVISION || (rev >= 0 && (rev as usize) < self.len()) {
             Some(rev)
         } else {
@@ -120,7 +138,7 b' const REVIDX_KNOWN_FLAGS: u16 = REVISION'
 
 const NULL_REVLOG_ENTRY_FLAGS: u16 = 0;
 
-#[derive(Debug, derive_more::From)]
+#[derive(Debug, derive_more::From, derive_more::Display)]
 pub enum RevlogError {
     InvalidRevision,
     /// Working directory is not supported
@@ -231,10 +249,11 b' impl Revlog {'
 
     /// Returns the node ID for the given revision number, if it exists in this
     /// revlog
-    pub fn node_from_rev(&self, rev: Revision) -> Option<&Node> {
-        if rev == NULL_REVISION {
+    pub fn node_from_rev(&self, rev: UncheckedRevision) -> Option<&Node> {
+        if rev == NULL_REVISION.into() {
             return Some(&NULL_NODE);
         }
+        let rev = self.index.check_revision(rev)?;
         Some(self.index.get_entry(rev)?.hash())
     }
 
@@ -296,8 +315,8 b' impl Revlog {'
     }
 
     /// Returns whether the given revision exists in this revlog.
-    pub fn has_rev(&self, rev: Revision) -> bool {
-        self.index.get_entry(rev).is_some()
+    pub fn has_rev(&self, rev: UncheckedRevision) -> bool {
+        self.index.check_revision(rev).is_some()
     }
 
     /// Return the full data associated to a revision.
@@ -307,12 +326,23 b' impl Revlog {'
     /// snapshot to rebuild the final data.
     pub fn get_rev_data(
         &self,
+        rev: UncheckedRevision,
+    ) -> Result<Cow<[u8]>, RevlogError> {
+        if rev == NULL_REVISION.into() {
+            return Ok(Cow::Borrowed(&[]));
+        };
+        self.get_entry(rev)?.data()
+    }
+
+    /// [`Self::get_rev_data`] for checked revisions.
+    pub fn get_rev_data_for_checked_rev(
+        &self,
         rev: Revision,
     ) -> Result<Cow<[u8]>, RevlogError> {
         if rev == NULL_REVISION {
             return Ok(Cow::Borrowed(&[]));
         };
-        Ok(self.get_entry(rev)?.data()?)
+        self.get_entry_for_checked_rev(rev)?.data()
     }
 
     /// Check the hash of some given data against the recorded hash.
@@ -380,8 +410,7 b' impl Revlog {'
         }
     }
 
-    /// Get an entry of the revlog.
-    pub fn get_entry(
+    fn get_entry_for_checked_rev(
         &self,
         rev: Revision,
     ) -> Result<RevlogEntry, RevlogError> {
@@ -399,36 +428,60 b' impl Revlog {'
         } else {
             &self.data()[start..end]
         };
+        let base_rev = self
+            .index
+            .check_revision(index_entry.base_revision_or_base_of_delta_chain())
+            .ok_or_else(|| {
+                RevlogError::corrupted(format!(
+                    "base revision for rev {} is invalid",
+                    rev
+                ))
+            })?;
+        let p1 =
+            self.index.check_revision(index_entry.p1()).ok_or_else(|| {
+                RevlogError::corrupted(format!(
+                    "p1 for rev {} is invalid",
+                    rev
+                ))
+            })?;
+        let p2 =
+            self.index.check_revision(index_entry.p2()).ok_or_else(|| {
+                RevlogError::corrupted(format!(
+                    "p2 for rev {} is invalid",
+                    rev
+                ))
+            })?;
         let entry = RevlogEntry {
             revlog: self,
             rev,
             bytes: data,
             compressed_len: index_entry.compressed_len(),
             uncompressed_len: index_entry.uncompressed_len(),
-            base_rev_or_base_of_delta_chain: if index_entry
-                .base_revision_or_base_of_delta_chain()
-                == rev
-            {
+            base_rev_or_base_of_delta_chain: if base_rev == rev {
                 None
             } else {
-                Some(index_entry.base_revision_or_base_of_delta_chain())
+                Some(base_rev)
             },
-            p1: index_entry.p1(),
-            p2: index_entry.p2(),
+            p1,
+            p2,
             flags: index_entry.flags(),
             hash: *index_entry.hash(),
         };
         Ok(entry)
     }
 
-    /// when resolving internal references within revlog, any errors
-    /// should be reported as corruption, instead of e.g. "invalid revision"
-    fn get_entry_internal(
+    /// Get an entry of the revlog.
+    pub fn get_entry(
         &self,
-        rev: Revision,
-    ) -> Result<RevlogEntry, HgError> {
-        self.get_entry(rev)
-            .map_err(|_| corrupted(format!("revision {} out of range", rev)))
+        rev: UncheckedRevision,
+    ) -> Result<RevlogEntry, RevlogError> {
+        if rev == NULL_REVISION.into() {
+            return Ok(self.make_null_entry());
+        }
+        let rev = self.index.check_revision(rev).ok_or_else(|| {
+            RevlogError::corrupted(format!("rev {} is invalid", rev))
+        })?;
+        self.get_entry_for_checked_rev(rev)
     }
 }
 
@@ -486,7 +539,7 b" impl<'revlog> RevlogEntry<'revlog> {"
         if self.p1 == NULL_REVISION {
             Ok(None)
         } else {
-            Ok(Some(self.revlog.get_entry(self.p1)?))
+            Ok(Some(self.revlog.get_entry_for_checked_rev(self.p1)?))
         }
     }
 
@@ -496,7 +549,7 b" impl<'revlog> RevlogEntry<'revlog> {"
         if self.p2 == NULL_REVISION {
             Ok(None)
         } else {
-            Ok(Some(self.revlog.get_entry(self.p2)?))
+            Ok(Some(self.revlog.get_entry_for_checked_rev(self.p2)?))
         }
     }
 
@@ -527,7 +580,7 b" impl<'revlog> RevlogEntry<'revlog> {"
     }
 
     /// The data for this entry, after resolving deltas if any.
-    pub fn rawdata(&self) -> Result<Cow<'revlog, [u8]>, HgError> {
+    pub fn rawdata(&self) -> Result<Cow<'revlog, [u8]>, RevlogError> {
         let mut entry = self.clone();
         let mut delta_chain = vec![];
 
@@ -539,11 +592,11 b" impl<'revlog> RevlogEntry<'revlog> {"
         while let Some(base_rev) = entry.base_rev_or_base_of_delta_chain {
             entry = if uses_generaldelta {
                 delta_chain.push(entry);
-                self.revlog.get_entry_internal(base_rev)?
+                self.revlog.get_entry_for_checked_rev(base_rev)?
             } else {
-                let base_rev = entry.rev - 1;
+                let base_rev = UncheckedRevision(entry.rev - 1);
                 delta_chain.push(entry);
-                self.revlog.get_entry_internal(base_rev)?
+                self.revlog.get_entry(base_rev)?
             };
         }
 
@@ -559,7 +612,7 b" impl<'revlog> RevlogEntry<'revlog> {"
     fn check_data(
         &self,
         data: Cow<'revlog, [u8]>,
-    ) -> Result<Cow<'revlog, [u8]>, HgError> {
+    ) -> Result<Cow<'revlog, [u8]>, RevlogError> {
         if self.revlog.check_hash(
             self.p1,
             self.p2,
@@ -571,22 +624,24 b" impl<'revlog> RevlogEntry<'revlog> {"
             if (self.flags & REVISION_FLAG_ELLIPSIS) != 0 {
                 return Err(HgError::unsupported(
                     "ellipsis revisions are not supported by rhg",
-                ));
+                )
+                .into());
             }
             Err(corrupted(format!(
                 "hash check failed for revision {}",
                 self.rev
-            )))
+            ))
+            .into())
         }
     }
 
-    pub fn data(&self) -> Result<Cow<'revlog, [u8]>, HgError> {
+    pub fn data(&self) -> Result<Cow<'revlog, [u8]>, RevlogError> {
         let data = self.rawdata()?;
         if self.rev == NULL_REVISION {
             return Ok(data);
         }
         if self.is_censored() {
-            return Err(HgError::CensoredNodeError);
+            return Err(HgError::CensoredNodeError.into());
         }
         self.check_data(data)
     }
@@ -705,13 +760,13 b' mod tests {'
         let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
         assert!(revlog.is_empty());
         assert_eq!(revlog.len(), 0);
-        assert!(revlog.get_entry(0).is_err());
-        assert!(!revlog.has_rev(0));
+        assert!(revlog.get_entry(0.into()).is_err());
+        assert!(!revlog.has_rev(0.into()));
         assert_eq!(
             revlog.rev_from_node(NULL_NODE.into()).unwrap(),
             NULL_REVISION
         );
-        let null_entry = revlog.get_entry(NULL_REVISION).ok().unwrap();
+        let null_entry = revlog.get_entry(NULL_REVISION.into()).ok().unwrap();
         assert_eq!(null_entry.revision(), NULL_REVISION);
         assert!(null_entry.data().unwrap().is_empty());
     }
@@ -750,7 +805,7 b' mod tests {'
         std::fs::write(temp.path().join("foo.i"), contents).unwrap();
         let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
 
-        let entry0 = revlog.get_entry(0).ok().unwrap();
+        let entry0 = revlog.get_entry(0.into()).ok().unwrap();
         assert_eq!(entry0.revision(), 0);
         assert_eq!(*entry0.node(), node0);
         assert!(!entry0.has_p1());
@@ -761,7 +816,7 b' mod tests {'
         let p2_entry = entry0.p2_entry().unwrap();
         assert!(p2_entry.is_none());
 
-        let entry1 = revlog.get_entry(1).ok().unwrap();
+        let entry1 = revlog.get_entry(1.into()).ok().unwrap();
         assert_eq!(entry1.revision(), 1);
         assert_eq!(*entry1.node(), node1);
         assert!(!entry1.has_p1());
@@ -772,7 +827,7 b' mod tests {'
         let p2_entry = entry1.p2_entry().unwrap();
         assert!(p2_entry.is_none());
 
-        let entry2 = revlog.get_entry(2).ok().unwrap();
+        let entry2 = revlog.get_entry(2.into()).ok().unwrap();
         assert_eq!(entry2.revision(), 2);
         assert_eq!(*entry2.node(), node2);
         assert!(entry2.has_p1());
@@ -817,7 +872,7 b' mod tests {'
         let revlog = Revlog::open(&vfs, "foo.i", None, false).unwrap();
 
         // accessing the data shows the corruption
-        revlog.get_entry(0).unwrap().data().unwrap_err();
+        revlog.get_entry(0.into()).unwrap().data().unwrap_err();
 
         assert_eq!(revlog.rev_from_node(NULL_NODE.into()).unwrap(), -1);
         assert_eq!(revlog.rev_from_node(node0.into()).unwrap(), 0);

rust/hg-core/src/revlog/nodemap.rs

@@ -12,6 +12,8 b''
 //! Following existing implicit conventions, the "nodemap" terminology
 //! is used in a more abstract context.
 
+use crate::UncheckedRevision;
+
 use super::{
     node::NULL_NODE, Node, NodePrefix, Revision, RevlogIndex, NULL_REVISION,
 };
@@ -30,7 +32,7 b' pub enum NodeMapError {'
     /// This can be returned by methods meant for (at most) one match.
     MultipleResults,
     /// A `Revision` stored in the nodemap could not be found in the index
-    RevisionNotInIndex(Revision),
+    RevisionNotInIndex(UncheckedRevision),
 }
 
 /// Mapping system from Mercurial nodes to revision numbers.
@@ -125,7 +127,9 b' type RawElement = unaligned::I32Be;'
 /// use.
 #[derive(Clone, Debug, Eq, PartialEq)]
 enum Element {
-    Rev(Revision),
+    // This is not a Mercurial revision. It's a `i32` because this is the
+    // right type for this structure.
+    Rev(i32),
     Block(usize),
     None,
 }
@@ -245,17 +249,21 b' impl Index<usize> for NodeTree {'
 fn has_prefix_or_none(
     idx: &impl RevlogIndex,
     prefix: NodePrefix,
-    rev: Revision,
+    rev: UncheckedRevision,
 ) -> Result<Option<Revision>, NodeMapError> {
-    idx.node(rev)
+    match idx.check_revision(rev) {
+        Some(checked) => idx
+            .node(checked)
         .ok_or(NodeMapError::RevisionNotInIndex(rev))
         .map(|node| {
             if prefix.is_prefix_of(node) {
-                Some(rev)
+                    Some(checked)
             } else {
                 None
             }
-        })
+            }),
+        None => Err(NodeMapError::RevisionNotInIndex(rev)),
+    }
 }
 
 /// validate that the candidate's node starts indeed with given prefix,
@@ -266,7 +274,7 b' fn has_prefix_or_none('
 fn validate_candidate(
     idx: &impl RevlogIndex,
     prefix: NodePrefix,
-    candidate: (Option<Revision>, usize),
+    candidate: (Option<UncheckedRevision>, usize),
 ) -> Result<(Option<Revision>, usize), NodeMapError> {
     let (rev, steps) = candidate;
     if let Some(nz_nybble) = prefix.first_different_nybble(&NULL_NODE) {
@@ -384,6 +392,8 b' impl NodeTree {'
     /// be inferred from
     /// the `NodeTree` data is that `rev` is the revision with the longest
     /// common node prefix with the given prefix.
+    /// We return an [`UncheckedRevision`] because we have no guarantee that
+    /// the revision we found is valid for the index.
     ///
     /// The second returned value is the size of the smallest subprefix
     /// of `prefix` that would give the same result, i.e. not the
@@ -392,7 +402,7 b' impl NodeTree {'
     fn lookup(
         &self,
         prefix: NodePrefix,
-    ) -> Result<(Option<Revision>, usize), NodeMapError> {
+    ) -> Result<(Option<UncheckedRevision>, usize), NodeMapError> {
         for (i, visit_item) in self.visit(prefix).enumerate() {
             if let Some(opt) = visit_item.final_revision() {
                 return Ok((opt, i + 1));
@@ -464,9 +474,9 b' impl NodeTree {'
             self.mutable_block(deepest.block_idx);
 
         if let Element::Rev(old_rev) = deepest.element {
-            let old_node = index
-                .node(old_rev)
-                .ok_or(NodeMapError::RevisionNotInIndex(old_rev))?;
+            let old_node = index.node(old_rev).ok_or_else(|| {
+                NodeMapError::RevisionNotInIndex(old_rev.into())
+            })?;
             if old_node == node {
                 return Ok(()); // avoid creating lots of useless blocks
             }
@@ -623,13 +633,13 b" impl<'n> Iterator for NodeTreeVisitor<'n"
 
 impl NodeTreeVisitItem {
     // Return `Some(opt)` if this item is final, with `opt` being the
-    // `Revision` that it may represent.
+    // `UncheckedRevision` that it may represent.
     //
     // If the item is not terminal, return `None`
-    fn final_revision(&self) -> Option<Option<Revision>> {
+    fn final_revision(&self) -> Option<Option<UncheckedRevision>> {
         match self.element {
             Element::Block(_) => None,
-            Element::Rev(r) => Some(Some(r)),
+            Element::Rev(r) => Some(Some(r.into())),
             Element::None => Some(None),
         }
     }
@@ -733,16 +743,20 b' mod tests {'
         assert_eq!(block.get(4), Element::Rev(1));
     }
 
-    type TestIndex = HashMap<Revision, Node>;
+    type TestIndex = HashMap<UncheckedRevision, Node>;
 
     impl RevlogIndex for TestIndex {
         fn node(&self, rev: Revision) -> Option<&Node> {
-            self.get(&rev)
+            self.get(&rev.into())
         }
 
         fn len(&self) -> usize {
             self.len()
         }
+
+        fn check_revision(&self, rev: UncheckedRevision) -> Option<Revision> {
+            self.get(&rev).map(|_| rev.0)
+        }
     }
 
     /// Pad hexadecimal Node prefix with zeros on the right
@@ -756,7 +770,7 b' mod tests {'
 
     /// Pad hexadecimal Node prefix with zeros on the right, then insert
     fn pad_insert(idx: &mut TestIndex, rev: Revision, hex: &str) {
-        idx.insert(rev, pad_node(hex));
+        idx.insert(rev.into(), pad_node(hex));
     }
 
     fn sample_nodetree() -> NodeTree {
@@ -796,7 +810,7 b' mod tests {'
         assert_eq!(nt.find_bin(&idx, hex("ab"))?, None);
 
         // and with full binary Nodes
-        assert_eq!(nt.find_node(&idx, idx.get(&1).unwrap())?, Some(1));
+        assert_eq!(nt.find_node(&idx, idx.get(&1.into()).unwrap())?, Some(1));
         let unknown = Node::from_hex(&hex_pad_right("3d")).unwrap();
         assert_eq!(nt.find_node(&idx, &unknown)?, None);
         Ok(())
@@ -857,14 +871,15 b' mod tests {'
             }
         }
 
-        fn insert(
-            &mut self,
-            rev: Revision,
-            hex: &str,
-        ) -> Result<(), NodeMapError> {
+        fn insert(&mut self, rev: i32, hex: &str) -> Result<(), NodeMapError> {
             let node = pad_node(hex);
+            let rev: UncheckedRevision = rev.into();
             self.index.insert(rev, node);
-            self.nt.insert(&self.index, &node, rev)?;
+            self.nt.insert(
+                &self.index,
+                &node,
+                self.index.check_revision(rev).unwrap(),
+            )?;
             Ok(())
         }
 
@@ -971,9 +986,9 b' mod tests {'
         let node0 = Node::from_hex(&node0_hex).unwrap();
         let node1 = Node::from_hex(&node1_hex).unwrap();
 
-        idx.insert(0, node0);
+        idx.insert(0.into(), node0);
         nt.insert(idx, &node0, 0)?;
-        idx.insert(1, node1);
+        idx.insert(1.into(), node1);
         nt.insert(idx, &node1, 1)?;
 
         assert_eq!(nt.find_bin(idx, (&node0).into())?, Some(0));

rust/hg-core/src/revset.rs

@@ -53,7 +53,7 b' pub fn resolve_rev_number_or_hex_prefix('
     if let Ok(integer) = input.parse::<i32>() {
         if integer.to_string() == input
             && integer >= 0
-            && revlog.has_rev(integer)
+            && revlog.has_rev(integer.into())
         {
             return Ok(integer);
         }

rust/hg-cpython/src/revlog.rs

@@ -18,7 +18,7 b' use cpython::{'
 use hg::{
     nodemap::{Block, NodeMapError, NodeTree},
     revlog::{nodemap::NodeMap, NodePrefix, RevlogIndex},
-    Revision,
+    Revision, UncheckedRevision,
 };
 use std::cell::RefCell;
 
@@ -252,7 +252,7 b' py_class!(pub class MixedIndex |py| {'
         // Note that we don't seem to have a direct way to call
         // PySequence_GetItem (does the job), which would possibly be better
         // for performance
-        let key = match key.extract::<Revision>(py) {
+        let key = match key.extract::<i32>(py) {
             Ok(rev) => rev.to_py_object(py).into_object(),
             Err(_) => key,
         };
@@ -268,7 +268,7 b' py_class!(pub class MixedIndex |py| {'
         // this is an equivalent implementation of the index_contains()
         // defined in revlog.c
         let cindex = self.cindex(py).borrow();
-        match item.extract::<Revision>(py) {
+        match item.extract::<i32>(py) {
             Ok(rev) => {
                 Ok(rev >= -1 && rev < cindex.inner().len(py)? as Revision)
             }
@@ -448,9 +448,12 b' impl MixedIndex {'
         let mut nt = NodeTree::load_bytes(Box::new(bytes), len);
 
         let data_tip =
-            docket.getattr(py, "tip_rev")?.extract::<Revision>(py)?;
+            docket.getattr(py, "tip_rev")?.extract::<i32>(py)?.into();
         self.docket(py).borrow_mut().replace(docket.clone_ref(py));
         let idx = self.cindex(py).borrow();
+        let data_tip = idx.check_revision(data_tip).ok_or_else(|| {
+            nodemap_error(py, NodeMapError::RevisionNotInIndex(data_tip))
+        })?;
         let current_tip = idx.len();
 
         for r in (data_tip + 1)..current_tip as Revision {
@@ -479,7 +482,7 b' fn revlog_error(py: Python) -> PyErr {'
     }
 }
 
-fn rev_not_in_index(py: Python, rev: Revision) -> PyErr {
+fn rev_not_in_index(py: Python, rev: UncheckedRevision) -> PyErr {
     PyErr::new::<ValueError, _>(
         py,
         format!(