D7798: rust-nodemap: special case for prefixes of NULL_NODE
gracinet created this revision. Herald added subscribers: mercurial-devel, kevincox, durin42. Herald added a reviewer: hg-reviewers. REVISION SUMMARY We have to behave as though NULL_NODE was stored in the node tree, although we don't store it. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,8 +13,8 @@ //! is used in a more abstract context. use super::{ -node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision, -RevlogIndex, +node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix, +NodePrefixRef, Revision, RevlogIndex, NULL_REVISION, }; use std::fmt; use std::mem; @@ -209,6 +209,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -282,9 +307,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks -/// - special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -519,9 +541,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -645,8 +665,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -665,7 +686,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
gracinet updated this revision to Diff 19138. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=19047&id=19138 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,8 +13,8 @@ //! is used in a more abstract context. use super::{ -node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision, -RevlogIndex, +node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix, +NodePrefixRef, Revision, RevlogIndex, NULL_REVISION, }; use std::fmt; use std::mem; @@ -209,6 +209,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -282,9 +307,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks -/// - special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -519,9 +541,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -645,8 +665,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -665,7 +686,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
gracinet updated this revision to Diff 19330. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=19138&id=19330 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,8 +13,8 @@ //! is used in a more abstract context. use super::{ -node::get_nybble, Node, NodeError, NodePrefix, NodePrefixRef, Revision, -RevlogIndex, +node::get_nybble, node::NULL_NODE, Node, NodeError, NodePrefix, +NodePrefixRef, Revision, RevlogIndex, NULL_REVISION, }; use std::fmt; use std::mem; @@ -207,6 +207,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -280,9 +305,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks -/// - special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -517,9 +539,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -643,8 +663,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -663,7 +684,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
gracinet updated this revision to Diff 19638. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=19330&id=19638 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -250,6 +251,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -330,8 +356,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -582,9 +606,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -713,8 +735,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -733,7 +756,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
marmoute added a comment. marmoute accepted this revision. looks good to me too. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
marmoute updated this revision to Diff 20027. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=19638&id=20027 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -250,6 +251,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -330,8 +356,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -582,9 +606,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -713,8 +735,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -733,7 +756,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
Alphare updated this revision to Diff 20217. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=20027&id=20217 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -250,6 +251,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate<'p>( +idx: &impl RevlogIndex, +prefix: NodePrefixRef<'p>, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -339,8 +365,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -591,9 +615,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -723,8 +745,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -743,7 +766,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
Alphare updated this revision to Diff 20236. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=20217&id=20236 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -268,6 +269,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate( +idx: &impl RevlogIndex, +prefix: NodePrefixRef, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -358,8 +384,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -610,9 +634,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -745,8 +767,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -765,7 +788,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
Alphare updated this revision to Diff 20282. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=20236&id=20282 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -270,6 +271,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate( +idx: &impl RevlogIndex, +prefix: NodePrefixRef, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -361,8 +387,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -613,9 +637,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -748,8 +770,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -768,7 +791,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7798: rust-nodemap: special case for prefixes of NULL_NODE
Closed by commit rHG895934342631: rust-nodemap: special case for prefixes of NULL_NODE (authored by gracinet). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7798?vs=20282&id=20312 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7798/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7798 AFFECTED FILES rust/hg-core/src/revlog/nodemap.rs CHANGE DETAILS diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs --- a/rust/hg-core/src/revlog/nodemap.rs +++ b/rust/hg-core/src/revlog/nodemap.rs @@ -13,7 +13,8 @@ //! is used in a more abstract context. use super::{ -Node, NodeError, NodePrefix, NodePrefixRef, Revision, RevlogIndex, +node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision, +RevlogIndex, NULL_REVISION, }; use std::fmt; @@ -270,6 +271,31 @@ }) } +/// validate that the candidate's node starts indeed with given prefix, +/// and treat ambiguities related to `NULL_REVISION`. +/// +/// From the data in the NodeTree, one can only conclude that some +/// revision is the only one for a *subprefix* of the one being looked up. +fn validate_candidate( +idx: &impl RevlogIndex, +prefix: NodePrefixRef, +rev: Option, +) -> Result, NodeMapError> { +if prefix.is_prefix_of(&NULL_NODE) { +// NULL_REVISION always matches a prefix made only of zeros +// and any other *valid* result is an ambiguity +match rev { +None => Ok(Some(NULL_REVISION)), +Some(r) => match has_prefix_or_none(idx, prefix, r)? { +None => Ok(Some(NULL_REVISION)), +_ => Err(NodeMapError::MultipleResults), +}, +} +} else { +rev.map_or(Ok(None), |r| has_prefix_or_none(idx, prefix, r)) +} +} + impl NodeTree { /// Initiate a NodeTree from an immutable slice-like of `Block` /// @@ -361,8 +387,6 @@ } /// Main working method for `NodeTree` searches -/// -/// This partial implementation lacks special cases for NULL_REVISION fn lookup<'p>( &self, prefix: NodePrefixRef<'p>, @@ -613,9 +637,7 @@ idx: &impl RevlogIndex, prefix: NodePrefixRef<'a>, ) -> Result, NodeMapError> { -self.lookup(prefix.clone()).and_then(|opt| { -opt.map_or(Ok(None), |rev| has_prefix_or_none(idx, prefix, rev)) -}) +validate_candidate(idx, prefix.clone(), self.lookup(prefix)?) } } @@ -748,8 +770,9 @@ assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9))); -assert_eq!(nt.find_hex(&idx, "00"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0))); +assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION))); } #[test] @@ -768,7 +791,8 @@ }; assert_eq!(nt.find_hex(&idx, "10")?, Some(1)); assert_eq!(nt.find_hex(&idx, "c")?, Some(2)); -assert_eq!(nt.find_hex(&idx, "00")?, Some(0)); +assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults)); +assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION)); assert_eq!(nt.find_hex(&idx, "01")?, Some(9)); Ok(()) } To: gracinet, #hg-reviewers, kevincox, marmoute Cc: marmoute, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel