Skip to content

Commit

Permalink
Address/Size data types refactor
Browse files Browse the repository at this point in the history 
Address and Size were not the abstractions I needed; what I needed were a rarely-used AbsoluteAddress and a more versatile Offset. Refactor entire codebase to use these new abstractions and stop converting willy nilly between them.
  • Loading branch information
@misson20000
misson20000 committed Nov 24, 2024
1 parent 514e820 commit 20f9d7d
Show file tree
Hide file tree
Showing 31 changed files with 732 additions and 780 deletions.
664 changes: 313 additions & 351 deletions src/model/addr.rs
View file

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions src/model/document.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ impl From<roxmltree::Error> for LoadForTestingError {
impl Builder {
pub fn default() -> Self {
Self::new(sync::Arc::new(structure::Node {
size: addr::unit::MAX,
size: addr::Offset::MAX,
props: structure::Properties {
name: "root".to_string(),
title_display: structure::TitleDisplay::Major,
Expand Down Expand Up @@ -119,13 +119,13 @@ impl Document {
})
}

pub fn lookup_node(&self, path: structure::PathSlice) -> (&sync::Arc<structure::Node>, addr::Address) {
pub fn lookup_node(&self, path: structure::PathSlice) -> (&sync::Arc<structure::Node>, addr::AbsoluteAddress) {
let mut current_node = &self.root;
let mut node_addr = addr::unit::NULL;
let mut node_addr = addr::AbsoluteAddress::NULL;

for i in path {
let childhood = &current_node.children[*i];
node_addr+= childhood.offset.to_size();
node_addr+= childhood.offset;
current_node = &childhood.node;
}

Expand All @@ -136,7 +136,7 @@ impl Document {
self.root.successor(path, 0)
}

pub fn search_addr<A: Into<addr::Address>>(&self, addr: A, traversal: search::Traversal) -> Result<search::AddressSearch<'_>, search::SetupError> {
pub fn search_addr<A: Into<addr::AbsoluteAddress>>(&self, addr: A, traversal: search::Traversal) -> Result<search::AddressSearch<'_>, search::SetupError> {
search::AddressSearch::new(self, addr.into(), traversal)
}

Expand Down
38 changes: 19 additions & 19 deletions src/model/document/change.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub enum ChangeType {
* information they need without having to look up nodes in the old
* document. */
num_grandchildren: usize,
offset: addr::Address
offset: addr::Offset
},

/// Deletes some of a node's (range inclusive) children.
Expand Down Expand Up @@ -376,7 +376,7 @@ impl Change {

/* Check offset to make sure it's within bounds. */
// TODO: automatically grow parents?
if childhood.offset > target.size.to_addr() {
if childhood.offset > target.size {
return Err(ApplyErrorType::InvalidParameters("attempted to insert node beginning beyond parent's size"));
}

Expand All @@ -387,7 +387,7 @@ impl Change {
None => return Err(ApplyErrorType::InvalidParameters("attempted to insert node at a place where its end would overflow"))
};

if end > target.size.to_addr() {
if end > target.size {
return Err(ApplyErrorType::InvalidParameters("attempted to insert node extending beyond parent's size"));
}

Expand Down Expand Up @@ -416,13 +416,13 @@ impl Change {
let mut children: Vec<structure::Childhood> = parent_node.children.splice(range.indices(), [structure::Childhood::default()]).collect();

for child in &mut children {
child.offset-= extent.begin.to_size();
child.offset-= extent.begin;
}

let new_node = &mut parent_node.children[range.first];
new_node.offset = extent.begin;
new_node.node = sync::Arc::new(structure::Node {
size: extent.length(),
size: extent.len(),
children: children,
props: props.clone(),
});
Expand Down Expand Up @@ -451,7 +451,7 @@ impl Change {
parent_node.children.splice(*child_index..*child_index, destructured_child.node.children.iter().map(|childhood| {
/* Don't forget to shift offsets. */
let mut childhood = childhood.clone();
childhood.offset+= offset.to_size();
childhood.offset+= *offset;
childhood
}));

Expand Down Expand Up @@ -619,28 +619,28 @@ mod tests {
let mut path = vec![1, 0, 2];

assert_eq!(Change {
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 1, child: structure::Node::builder().build_child(addr::unit::NULL) },
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 1, child: structure::Node::builder().build_child(addr::Offset::NULL) },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Moved);

assert_eq!(path, vec![1, 0, 3]);

assert_eq!(Change {
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 4, child: structure::Node::builder().build_child(addr::unit::NULL) },
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 4, child: structure::Node::builder().build_child(addr::Offset::NULL) },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Unmoved);

assert_eq!(path, vec![1, 0, 3]);

assert_eq!(Change {
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 3, child: structure::Node::builder().build_child(addr::unit::NULL) },
ty: ChangeType::InsertNode { parent: vec![1, 0], index: 3, child: structure::Node::builder().build_child(addr::Offset::NULL) },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Moved);

assert_eq!(path, vec![1, 0, 4]);

assert_eq!(Change {
ty: ChangeType::InsertNode { parent: vec![1, 1], index: 0, child: structure::Node::builder().build_child(addr::unit::NULL) },
ty: ChangeType::InsertNode { parent: vec![1, 1], index: 0, child: structure::Node::builder().build_child(addr::Offset::NULL) },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Unmoved);

Expand All @@ -649,7 +649,7 @@ mod tests {

#[test]
fn test_update_path_through_nest() {
let extent = addr::Extent::between(addr::unit::NULL, addr::unit::NULL);
let extent = addr::Extent::between(addr::Offset::NULL, addr::Offset::NULL);

/* siblings before but not including the path */
let mut path = vec![1, 0, 2];
Expand Down Expand Up @@ -697,23 +697,23 @@ mod tests {
/* sibling of destructured node (before) */
let mut path = vec![1, 0];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Unmoved);
assert_eq!(path, vec![1, 0]);

/* descendant of sibling of destructured node (before) */
let mut path = vec![1, 0, 6, 7];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Unmoved);
assert_eq!(path, vec![1, 0, 6, 7]);

/* sibling of destructured node (after) */
let mut path = vec![1, 2];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Moved);
/* 1 before: [0, [0, 1, 2], 2, 3, ...] */
Expand All @@ -723,31 +723,31 @@ mod tests {
/* descendant of sibling of destructured node (after) */
let mut path = vec![1, 2, 6, 7];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Moved);
assert_eq!(path, vec![1, 4, 6, 7]);

/* destructured node */
let mut path = vec![1, 1];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Destructured);
assert_eq!(path, vec![1]);

/* descendant of destructured node */
let mut path = vec![1, 1, 2, 4];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Moved);
assert_eq!(path, vec![1, 3, 4]);

/* unrelated path */
let mut path = vec![2, 2, 6, 7];
assert_eq!(Change {
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::unit::NULL },
ty: ChangeType::Destructure { parent: vec![1], child_index: 1, num_grandchildren: 3, offset: addr::Offset::NULL },
generation: 0,
}.update_path(&mut path), UpdatePathResult::Unmoved);
assert_eq!(path, vec![2, 2, 6, 7]);
Expand Down Expand Up @@ -981,7 +981,7 @@ mod tests {
{
let mut doc = document::Builder::new(structure::Node::builder()
.name("root")
.size(addr::unit::MAX)
.size(addr::Offset::MAX)
.build()).build();
assert_matches!(Change {
ty: ChangeType::InsertNode { parent: vec![], index: 0, child: builder.build_child(0xfffffffffffffff8) },
Expand Down
18 changes: 9 additions & 9 deletions src/model/document/search.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ pub enum Traversal {
#[derive(Debug, Clone)]
struct TraversalStackEntry<'a> {
node: &'a sync::Arc<structure::Node>,
node_addr: addr::Address,
node_addr: addr::AbsoluteAddress,
is_leaf: bool,
child: Option<usize>,
}

#[derive(Debug, Clone)]
pub struct AddressSearch<'a> {
document: &'a document::Document,
needle: addr::Address,
needle: addr::AbsoluteAddress,
traversal: Traversal,

stack: Vec<TraversalStackEntry<'a>>,
Expand All @@ -31,7 +31,7 @@ pub struct AddressSearch<'a> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Hit {
pub path: structure::Path,
pub offset: addr::Size,
pub offset: addr::Offset,
}

#[derive(Debug, Clone, Copy)]
Expand All @@ -40,8 +40,8 @@ pub enum SetupError {
}

impl<'a> AddressSearch<'a> {
pub fn new(document: &'a document::Document, needle: addr::Address, traversal: Traversal) -> Result<AddressSearch<'a>, SetupError> {
if needle >= addr::unit::NULL + document.root.size {
pub fn new(document: &'a document::Document, needle: addr::AbsoluteAddress, traversal: Traversal) -> Result<AddressSearch<'a>, SetupError> {
if needle >= addr::AbsoluteAddress::NULL + document.root.size {
return Err(SetupError::NeedleNotInHaystack);
}

Expand All @@ -52,7 +52,7 @@ impl<'a> AddressSearch<'a> {

stack: vec![TraversalStackEntry {
node: &document.root,
node_addr: addr::unit::NULL,
node_addr: addr::AbsoluteAddress::NULL,
is_leaf: true,
child: None,
}],
Expand Down Expand Up @@ -81,8 +81,8 @@ impl<'a> AddressSearch<'a> {
}
*/

fn child_contains(child: &structure::Childhood, parent_node_addr: addr::Address, needle: addr::Address) -> bool {
addr::Extent::sized(parent_node_addr + child.offset.to_size(), child.node.size).includes(needle)
fn child_contains(child: &structure::Childhood, parent_node_addr: addr::AbsoluteAddress, needle: addr::AbsoluteAddress) -> bool {
addr::Extent::sized(parent_node_addr + child.offset, child.node.size).includes(needle)
}

}
Expand Down Expand Up @@ -118,7 +118,7 @@ impl<'a> Iterator for AddressSearch<'a> {

let entry = TraversalStackEntry {
node: &state.node.children[old_index].node,
node_addr: state.node_addr + state.node.children[old_index].offset.to_size(),
node_addr: state.node_addr + state.node.children[old_index].offset,
is_leaf: true,
child: None,
};
Expand Down
32 changes: 16 additions & 16 deletions src/model/document/structure.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ pub enum ChildrenDisplay {
pub enum ContentDisplay {
None,
Hexdump {
line_pitch: addr::Size,
gutter_pitch: addr::Size,
line_pitch: addr::Offset,
gutter_pitch: addr::Offset,
},
Hexstring
}
Expand Down Expand Up @@ -99,15 +99,15 @@ impl SiblingRange {
#[derive(Debug, Clone)]
pub struct Childhood {
pub node: sync::Arc<Node>,
pub offset: addr::Address,
pub offset: addr::Offset,
}

impl Childhood {
pub fn new(node: sync::Arc<Node>, offset: addr::Address) -> Childhood {
pub fn new(node: sync::Arc<Node>, offset: addr::Offset) -> Childhood {
Childhood { node, offset }
}

pub fn end(&self) -> addr::Address {
pub fn end(&self) -> addr::Offset {
self.offset + self.node.size
}

Expand Down Expand Up @@ -143,7 +143,7 @@ pub struct Node {
/* reference to parent causes a lot of problems, so we don't have one and
opt to refer to nodes by path when necessary. */
pub props: Properties,
pub size: addr::Size,
pub size: addr::Offset,
pub children: vec::Vec<Childhood>
}

Expand Down Expand Up @@ -178,7 +178,7 @@ impl Default for ChildrenDisplay {
}

impl ContentDisplay {
pub fn preferred_pitch(&self) -> Option<addr::Size> {
pub fn preferred_pitch(&self) -> Option<addr::Offset> {
match self {
ContentDisplay::None => None,
ContentDisplay::Hexdump { line_pitch, .. } => Some(*line_pitch),
Expand All @@ -189,8 +189,8 @@ impl ContentDisplay {
/// An alternative to default() that explicitly returns a hexdump style.
pub fn default_hexdump() -> ContentDisplay {
ContentDisplay::Hexdump {
line_pitch: addr::Size::from(16),
gutter_pitch: addr::Size::from(8),
line_pitch: addr::Offset::from(16),
gutter_pitch: addr::Offset::from(8),
}
}
}
Expand All @@ -217,7 +217,7 @@ impl Default for Node {
fn default() -> Node {
Node {
props: Properties::default(),
size: addr::unit::MAX,
size: addr::Offset::MAX,
children: vec::Vec::new(),
}
}
Expand All @@ -232,13 +232,13 @@ impl Default for Childhood {

Childhood {
node: DEFAULT_NODE.clone(),
offset: addr::unit::NULL,
offset: addr::Offset::ZERO,
}
}
}

impl Node {
pub fn default_sized(size: addr::Size) -> Node {
pub fn default_sized(size: addr::Offset) -> Node {
Node {
props: Properties::default(),
size,
Expand All @@ -250,7 +250,7 @@ impl Node {
builder::StructureBuilder::default()
}

pub fn child_at_offset(&self, offset: addr::Address) -> usize {
pub fn child_at_offset(&self, offset: addr::Offset) -> usize {
self.children.partition_point(|ch| ch.offset < offset)
}

Expand Down Expand Up @@ -430,12 +430,12 @@ pub mod builder {
self
}

pub fn size<S: Into<addr::Size>>(mut self, size: S) -> Self {
pub fn size<S: Into<addr::Offset>>(mut self, size: S) -> Self {
self.node.size = size.into();
self
}

pub fn child<A: Into<addr::Address>, F: FnOnce(StructureBuilder) -> StructureBuilder>(mut self, offset: A, builder: F) -> Self {
pub fn child<A: Into<addr::Offset>, F: FnOnce(StructureBuilder) -> StructureBuilder>(mut self, offset: A, builder: F) -> Self {
self.node.children.push(Childhood {
offset: offset.into(),
node: builder(Self::default()).build()
Expand All @@ -447,7 +447,7 @@ pub mod builder {
sync::Arc::new(self.node.clone())
}

pub fn build_child<T: Into<addr::Address>>(&self, offset: T) -> Childhood {
pub fn build_child<T: Into<addr::Offset>>(&self, offset: T) -> Childhood {
Childhood::new(sync::Arc::new(self.node.clone()), offset.into())
}
}
Expand Down
Loading

0 comments on commit 20f9d7d

Please sign in to comment.