Mercurial > public > mercurial-scm > hg-stable
diff rust/hg-core/src/revlog/nodemap.rs @ 50988:1928b770e3e7
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.
author | Rapha?l Gom?s <rgomes@octobus.net> |
---|---|
date | Thu, 10 Aug 2023 11:00:34 +0200 |
parents | 3894763d92f8 |
children | 4c5f6e95df84 |
line wrap: on
line diff
--- a/rust/hg-core/src/revlog/nodemap.rs Mon Sep 11 11:52:33 2023 +0200 +++ b/rust/hg-core/src/revlog/nodemap.rs Thu Aug 10 11:00:34 2023 +0200 @@ -12,6 +12,8 @@ //! 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 @@ /// 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 @@ /// 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 @@ fn has_prefix_or_none( idx: &impl RevlogIndex, prefix: NodePrefix, - rev: Revision, + rev: UncheckedRevision, ) -> Result<Option<Revision>, NodeMapError> { - idx.node(rev) - .ok_or(NodeMapError::RevisionNotInIndex(rev)) - .map(|node| { - if prefix.is_prefix_of(node) { - Some(rev) - } else { - None - } - }) + match idx.check_revision(rev) { + Some(checked) => idx + .node(checked) + .ok_or(NodeMapError::RevisionNotInIndex(rev)) + .map(|node| { + if prefix.is_prefix_of(node) { + Some(checked) + } else { + None + } + }), + None => Err(NodeMapError::RevisionNotInIndex(rev)), + } } /// validate that the candidate's node starts indeed with given prefix, @@ -266,7 +274,7 @@ 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 @@ /// 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ /// 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 @@ 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 @@ } } - 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 @@ 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));