Replace justified unwraps in shardtree with expects
#119
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,9 +46,11 @@ where | |
| let _ = cache.put_cap(backend.get_cap()?); | ||
|
|
||
| backend.with_checkpoints(backend.checkpoint_count()?, |checkpoint_id, checkpoint| { | ||
| // TODO: Once MSRV is at least 1.82, replace this (and similar `expect()`s below) with: | ||
| // `let Ok(_) = cache.add_checkpoint(checkpoint_id.clone(), checkpoint.clone());` | ||
| cache | ||
| .add_checkpoint(checkpoint_id.clone(), checkpoint.clone()) | ||
| .expect("error type is Infallible"); | ||
| Ok(()) | ||
| })?; | ||
|
|
||
|
|
@@ -74,26 +76,37 @@ where | |
| } | ||
| self.deferred_actions.clear(); | ||
|
|
||
| for shard_root in self | ||
| .cache | ||
| .get_shard_roots() | ||
| .expect("error type is Infallible") | ||
| { | ||
| self.backend.put_shard( | ||
| self.cache | ||
| .get_shard(shard_root) | ||
| .expect("error type is Infallible") | ||
| .expect("known address"), | ||
| )?; | ||
| } | ||
| self.backend | ||
| .put_cap(self.cache.get_cap().expect("error type is Infallible"))?; | ||
|
|
||
| let mut checkpoints = Vec::with_capacity( | ||
| self.cache | ||
| .checkpoint_count() | ||
| .expect("error type is Infallible"), | ||
| ); | ||
| self.cache | ||
| .with_checkpoints( | ||
| self.cache | ||
| .checkpoint_count() | ||
| .expect("error type is Infallible"), | ||
| |checkpoint_id, checkpoint| { | ||
| checkpoints.push((checkpoint_id.clone(), checkpoint.clone())); | ||
| Ok(()) | ||
| }, | ||
| ) | ||
| .expect("error type is Infallible"); | ||
| for (checkpoint_id, checkpoint) in checkpoints { | ||
| self.backend.add_checkpoint(checkpoint_id, checkpoint)?; | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -197,9 +197,35 @@ pub struct LocatedTree<A, V> { | |
| } | ||
|
|
||
| impl<A, V> LocatedTree<A, V> { | ||
| /// Constructs a new LocatedTree from its constituent parts. | ||
| /// | ||
| /// Returns `None` if `root_addr` is inconsistent with `root` (in particular, if the | ||
| /// level of `root_addr` is too small to contain `tree`). | ||
| pub fn from_parts(root_addr: Address, root: Tree<A, V>) -> Option<Self> { | ||
| // In order to meet various pre-conditions throughout the crate, we require that | ||
| // no `Node::Parent` in `root` has a level of 0 relative to `root_addr`. | ||
| fn is_consistent<A, V>(addr: Address, root: &Tree<A, V>) -> bool { | ||
| match (&root.0, addr.children()) { | ||
| // Found an inconsistency! | ||
| (Node::Parent { .. }, None) => false, | ||
| // Check consistency of children recursively. | ||
| (Node::Parent { left, right, .. }, Some((l_addr, r_addr))) => { | ||
| is_consistent(l_addr, left) && is_consistent(r_addr, right) | ||
| } | ||
|
|
||
| // Leaves are technically allowed to occur at any level, so we do not | ||
| // require `addr` to have no children. | ||
| (Node::Leaf { .. }, _) => true, | ||
|
|
||
| // Nil nodes have no information, so we cannot verify that the data it | ||
| // represents is consistent with `root_addr`. Instead we rely on methods | ||
| // that mutate `LocatedTree` to verify that the insertion address is not | ||
| // inconsistent with `root_addr`. | ||
| (Node::Nil, _) => true, | ||
| } | ||
| } | ||
|
|
||
| is_consistent(root_addr, &root).then_some(LocatedTree { root_addr, root }) | ||
| } | ||
|
|
||
| /// Returns the root address of this tree. | ||
|
|
@@ -234,10 +260,13 @@ impl<A, V> LocatedTree<A, V> { | |
|
|
||
| /// Returns the value at the specified position, if any. | ||
| pub fn value_at_position(&self, position: Position) -> Option<&V> { | ||
| /// Pre-condition: `addr` must be the address of `root`. | ||
| fn go<A, V>(pos: Position, addr: Address, root: &Tree<A, V>) -> Option<&V> { | ||
| match &root.0 { | ||
| Node::Parent { left, right, .. } => { | ||
| let (l_addr, r_addr) = addr | ||
| .children() | ||
| .expect("has children because we checked `root` is a parent"); | ||
| if l_addr.position_range().contains(&pos) { | ||
| go(pos, l_addr, left) | ||
| } else { | ||
|
|
@@ -306,6 +335,7 @@ impl<A: Default + Clone, V: Clone> LocatedTree<A, V> { | |
| /// if the tree is terminated by a [`Node::Nil`] or leaf node before the specified address can | ||
| /// be reached. | ||
| pub fn subtree(&self, addr: Address) -> Option<Self> { | ||
| /// Pre-condition: `root_addr` must be the address of `root`. | ||
| fn go<A: Clone, V: Clone>( | ||
| root_addr: Address, | ||
| root: &Tree<A, V>, | ||
|
|
@@ -319,7 +349,9 @@ impl<A: Default + Clone, V: Clone> LocatedTree<A, V> { | |
| } else { | ||
| match &root.0 { | ||
| Node::Parent { left, right, .. } => { | ||
| let (l_addr, r_addr) = root_addr | ||
| .children() | ||
| .expect("has children because we checked `root` is a parent"); | ||
| if l_addr.contains(&addr) { | ||
| go(l_addr, left.as_ref(), addr) | ||
| } else { | ||
|
|
@@ -343,6 +375,7 @@ impl<A: Default + Clone, V: Clone> LocatedTree<A, V> { | |
| /// If this root address of this tree is lower down in the tree than the level specified, | ||
| /// the entire tree is returned as the sole element of the result vector. | ||
| pub fn decompose_to_level(self, level: Level) -> Vec<Self> { | ||
| /// Pre-condition: `root_addr` must be the address of `root`. | ||
| fn go<A: Clone, V: Clone>( | ||
| level: Level, | ||
| root_addr: Address, | ||
|
|
@@ -353,7 +386,9 @@ impl<A: Default + Clone, V: Clone> LocatedTree<A, V> { | |
| } else { | ||
| match root.0 { | ||
| Node::Parent { left, right, .. } => { | ||
| let (l_addr, r_addr) = root_addr | ||
| .children() | ||
| .expect("has children because we checked `root` is a parent"); | ||
| let mut l_decomposed = go( | ||
| level, | ||
| l_addr, | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pre-condition (and all similar others in this PR) is verifiably upheld:
go(subtree.root_addr, &subtree.root); it is a programming error for these to be inconsistent.go(r_addr, right)orgo(l_addr, left)with values obtained fromroot_addrandroot.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I note that the public API
LocatedTree::from_partsdoes not enforce that its arguments are consistent; we could potentially strengthen this by making that method fallible and checking that the actual encoded depth of the provided tree matches the provided root address (which would ensure that all parents within the provided tree correspond to a subtree node of the provided root address that can have children).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated this PR to add the consistency check to
LocatedTree::from_parts.