From 41b9bd130c5b7e79de4431c67fa2fe81627a3f5a Mon Sep 17 00:00:00 2001 From: Rigidity <35380458+Rigidity@users.noreply.github.com> Date: Thu, 10 Aug 2023 09:43:08 -0400 Subject: [PATCH] Convert NodePtr to a newtype rather than alias (#333) --- src/allocator.rs | 31 ++++++++++++++-------------- src/op_utils.rs | 12 +++++------ src/serde/object_cache.rs | 18 ++++++++-------- tools/src/bin/benchmark-clvm-cost.rs | 2 +- 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/allocator.rs b/src/allocator.rs index 9e8b79fb..2e1bc930 100644 --- a/src/allocator.rs +++ b/src/allocator.rs @@ -3,7 +3,8 @@ use crate::number::{node_from_number, number_from_u8, Number}; use crate::reduction::EvalErr; use bls12_381::{G1Affine, G1Projective, G2Affine, G2Projective}; -pub type NodePtr = i32; +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct NodePtr(pub i32); pub enum SExp { Atom, @@ -140,7 +141,7 @@ impl Allocator { self.u8_vec.extend_from_slice(v); let end = self.u8_vec.len() as u32; self.atom_vec.push(AtomBuf { start, end }); - Ok(-(self.atom_vec.len() as i32)) + Ok(NodePtr(-(self.atom_vec.len() as i32))) } pub fn new_number(&mut self, v: Number) -> Result { @@ -163,17 +164,17 @@ impl Allocator { return err(self.null(), "too many pairs"); } self.pair_vec.push(IntPair { first, rest }); - Ok(r) + Ok(NodePtr(r)) } pub fn new_substr(&mut self, node: NodePtr, start: u32, end: u32) -> Result { - if node >= 0 { + if node.0 >= 0 { return err(node, "(internal error) substr expected atom, got pair"); } if self.atom_vec.len() == self.atom_limit { return err(self.null(), "too many atoms"); } - let atom = self.atom_vec[(-node - 1) as usize]; + let atom = self.atom_vec[(-node.0 - 1) as usize]; let atom_len = atom.end - atom.start; if start > atom_len { return err(node, "substr start out of bounds"); @@ -188,7 +189,7 @@ impl Allocator { start: atom.start + start, end: atom.start + end, }); - Ok(-(self.atom_vec.len() as i32)) + Ok(NodePtr(-(self.atom_vec.len() as i32))) } pub fn new_concat(&mut self, new_size: usize, nodes: &[NodePtr]) -> Result { @@ -203,12 +204,12 @@ impl Allocator { let mut counter: usize = 0; for node in nodes { - if *node >= 0 { + if node.0 >= 0 { self.u8_vec.truncate(start); return err(*node, "(internal error) concat expected atom, got pair"); } - let term = self.atom_vec[(-node - 1) as usize]; + let term = self.atom_vec[(-node.0 - 1) as usize]; if counter + term.len() > new_size { self.u8_vec.truncate(start); return err(*node, "(internal error) concat passed invalid new_size"); @@ -229,7 +230,7 @@ impl Allocator { start: (start as u32), end, }); - Ok(-(self.atom_vec.len() as i32)) + Ok(NodePtr(-(self.atom_vec.len() as i32))) } pub fn atom_eq(&self, lhs: NodePtr, rhs: NodePtr) -> bool { @@ -237,8 +238,8 @@ impl Allocator { } pub fn atom(&self, node: NodePtr) -> &[u8] { - assert!(node < 0, "expected atom, got pair"); - let atom = self.atom_vec[(-node - 1) as usize]; + assert!(node.0 < 0, "expected atom, got pair"); + let atom = self.atom_vec[(-node.0 - 1) as usize]; &self.u8_vec[atom.start as usize..atom.end as usize] } @@ -289,8 +290,8 @@ impl Allocator { } pub fn sexp(&self, node: NodePtr) -> SExp { - if node >= 0 { - let pair = self.pair_vec[node as usize]; + if node.0 >= 0 { + let pair = self.pair_vec[node.0 as usize]; SExp::Pair(pair.first, pair.rest) } else { SExp::Atom @@ -310,11 +311,11 @@ impl Allocator { } pub fn null(&self) -> NodePtr { - -1 + NodePtr(-1) } pub fn one(&self) -> NodePtr { - -2 + NodePtr(-2) } #[cfg(feature = "counters")] diff --git a/src/op_utils.rs b/src/op_utils.rs index 0fae2cbe..c057685c 100644 --- a/src/op_utils.rs +++ b/src/op_utils.rs @@ -19,7 +19,7 @@ pub fn get_args( ) -> Result<[NodePtr; N], EvalErr> { let mut next = args; let mut counter = 0; - let mut ret: [NodePtr; N] = [0; N]; + let mut ret: [NodePtr; N] = [NodePtr(0); N]; while let Some((first, rest)) = a.next(next) { next = rest; @@ -92,7 +92,7 @@ pub fn get_varargs( ) -> Result<([NodePtr; N], usize), EvalErr> { let mut next = args; let mut counter = 0; - let mut ret: [NodePtr; N] = [0; N]; + let mut ret: [NodePtr; N] = [NodePtr(0); N]; while let Some((first, rest)) = a.next(next) { next = rest; @@ -132,19 +132,19 @@ fn test_get_varargs() { ); assert_eq!( get_varargs::<4>(&a, args3, "test").unwrap(), - ([a1, a2, a3, 0], 3) + ([a1, a2, a3, NodePtr(0)], 3) ); assert_eq!( get_varargs::<4>(&a, args2, "test").unwrap(), - ([a2, a3, 0, 0], 2) + ([a2, a3, NodePtr(0), NodePtr(0)], 2) ); assert_eq!( get_varargs::<4>(&a, args1, "test").unwrap(), - ([a3, 0, 0, 0], 1) + ([a3, NodePtr(0), NodePtr(0), NodePtr(0)], 1) ); assert_eq!( get_varargs::<4>(&a, args0, "test").unwrap(), - ([0, 0, 0, 0], 0) + ([NodePtr(0), NodePtr(0), NodePtr(0), NodePtr(0)], 0) ); let r = get_varargs::<3>(&a, args4, "test").unwrap_err(); diff --git a/src/serde/object_cache.rs b/src/serde/object_cache.rs index 141c21d1..0ee3bd5a 100644 --- a/src/serde/object_cache.rs +++ b/src/serde/object_cache.rs @@ -30,11 +30,11 @@ pub struct ObjectCache<'a, T> { /// and negative values become odd indices. fn node_to_index(node: &NodePtr) -> usize { - let node = *node; - if node < 0 { - (-node - node - 1) as usize + let value = node.0; + if value < 0 { + (-value - value - 1) as usize } else { - (node + node) as usize + (value + value) as usize } } @@ -264,11 +264,11 @@ fn test_serialized_length() { #[test] fn test_node_to_index() { - assert_eq!(node_to_index(&0), 0); - assert_eq!(node_to_index(&1), 2); - assert_eq!(node_to_index(&2), 4); - assert_eq!(node_to_index(&-1), 1); - assert_eq!(node_to_index(&-2), 3); + assert_eq!(node_to_index(&NodePtr(0)), 0); + assert_eq!(node_to_index(&NodePtr(1)), 2); + assert_eq!(node_to_index(&NodePtr(2)), 4); + assert_eq!(node_to_index(&NodePtr(-1)), 1); + assert_eq!(node_to_index(&NodePtr(-2)), 3); } // this test takes a very long time (>60s) in debug mode, so it only runs in release mode diff --git a/tools/src/bin/benchmark-clvm-cost.rs b/tools/src/bin/benchmark-clvm-cost.rs index d42f33ce..3d720004 100644 --- a/tools/src/bin/benchmark-clvm-cost.rs +++ b/tools/src/bin/benchmark-clvm-cost.rs @@ -16,7 +16,7 @@ enum OpArgs { // special argument to indicate it should be substituted for varied in the FreeBytes test to // measure cost per byte -const VARIABLE: NodePtr = 999; +const VARIABLE: NodePtr = NodePtr(999); // builds calls in the form: // ( arg arg ...)