From 15e0b5339fff92ce8c4278d6e2a570a70bea7f78 Mon Sep 17 00:00:00 2001 From: Alex Shelkovnykov Date: Tue, 24 Oct 2023 15:15:05 -0600 Subject: [PATCH] Replace non_unifying_equality by rewriting assert_jer_err --- rust/ares/src/interpreter.rs | 2 +- rust/ares/src/jets.rs | 39 +++++++++-- rust/ares/src/mem.rs | 123 ----------------------------------- rust/ares/src/noun.rs | 8 +-- 4 files changed, 35 insertions(+), 137 deletions(-) diff --git a/rust/ares/src/interpreter.rs b/rust/ares/src/interpreter.rs index 8dc14d4..58aaa98 100644 --- a/rust/ares/src/interpreter.rs +++ b/rust/ares/src/interpreter.rs @@ -264,7 +264,7 @@ pub struct Context { pub scry_stack: Noun, } -#[derive(Clone, Copy, Debug, PartialEq)] +#[derive(Clone, Copy, Debug)] pub enum Error { ScryBlocked(Noun), // path ScryCrashed(Noun), // trace diff --git a/rust/ares/src/jets.rs b/rust/ares/src/jets.rs index db4c945..34aa37d 100644 --- a/rust/ares/src/jets.rs +++ b/rust/ares/src/jets.rs @@ -39,7 +39,7 @@ pub type Jet = fn(&mut Context, Noun) -> Result; * Only return a deterministic error if the Nock would have deterministically * crashed. */ -#[derive(Debug, PartialEq)] +#[derive(Clone, Copy, Debug)] pub enum JetErr { Punt, // Retry with the raw nock Fail(Error), // Error; do not retry @@ -374,11 +374,38 @@ pub mod util { &jet_res ); let jet_err = jet_res.unwrap_err(); - assert_eq!( - jet_err, err, - "with sample: {}, expected err: {:?}, got: {:?}", - sam, err, jet_err - ); + match (jet_err, err) { + (JetErr::Punt, JetErr::Punt) => {} + (JetErr::Fail(actual_err), JetErr::Fail(expected_err)) => { + match (actual_err, expected_err) { + (Error::ScryBlocked(mut actual), Error::ScryBlocked(mut expected)) + | (Error::ScryCrashed(mut actual), Error::ScryCrashed(mut expected)) + | (Error::Deterministic(mut actual), Error::Deterministic(mut expected)) + | ( + Error::NonDeterministic(mut actual), + Error::NonDeterministic(mut expected), + ) => unsafe { + assert!(unifying_equality( + &mut context.stack, + &mut actual, + &mut expected + )); + }, + _ => { + panic!( + "with sample: {}, expected err: {:?}, got: {:?}", + sam, expected_err, actual_err + ); + } + } + } + _ => { + panic!( + "with sample: {}, expected err: {:?}, got: {:?}", + sam, err, jet_err + ); + } + } } } diff --git a/rust/ares/src/mem.rs b/rust/ares/src/mem.rs index a53f882..90cb32e 100644 --- a/rust/ares/src/mem.rs +++ b/rust/ares/src/mem.rs @@ -979,86 +979,6 @@ pub unsafe fn unifying_equality(stack: &mut NockStack, a: *mut Noun, b: *mut Nou (*a).raw_equals(*b) } -/// This function only exists as a default equality comparator for Nouns in cases where we don't -/// have a NockStack available to perform unifying equality checks. -/// -/// One such case is for the implementation of PartialEq on Noun, which is used by the derived -/// implementationa of PartialEq for interpreter::Error and jets::JetErr. These themselves are -/// only used during unit testing. -/// -/// This function is incredibly inefficient in the case of densely connected DAGs, and should -/// never be used without a very good reason. ALWAYS prefer unifying_equality. -pub fn non_unifying_equality(a: Noun, b: Noun) -> bool { - // If the nouns are already word-equal we have nothing to do - if unsafe { a.raw_equals(b) } { - return true; - }; - // If the nouns have cached mugs which are disequal we have nothing to do - if let (Ok(a_alloc), Ok(b_alloc)) = (a.as_allocated(), b.as_allocated()) { - if let (Some(a_mug), Some(b_mug)) = (a_alloc.get_cached_mug(), b_alloc.get_cached_mug()) { - if a_mug != b_mug { - return false; - }; - }; - }; - let mut stack = Vec::new(); - stack.push((a, b)); - while let Some((x, y)) = stack.pop() { - if unsafe { x.raw_equals(y) } { - continue; - }; - if let (Ok(x_alloc), Ok(y_alloc)) = ( - // equal direct atoms return true for raw_equals() - x.as_allocated(), - y.as_allocated(), - ) { - if let (Some(x_mug), Some(y_mug)) = (x_alloc.get_cached_mug(), y_alloc.get_cached_mug()) - { - if x_mug != y_mug { - // short-circuit: the mugs differ therefore the nouns must differ - return false; - } - }; - match (x_alloc.as_either(), y_alloc.as_either()) { - (Left(x_indirect), Left(y_indirect)) => { - if unsafe { - x_indirect.size() == y_indirect.size() - && memcmp( - x_indirect.data_pointer() as *const c_void, - y_indirect.data_pointer() as *const c_void, - x_indirect.size() << 3, - ) == 0 - } { - continue; - } else { - return false; - } - } - (Right(x_cell), Right(y_cell)) => { - if unsafe { - x_cell.head().raw_equals(y_cell.head()) - && x_cell.tail().raw_equals(y_cell.tail()) - } { - continue; - } else { - stack.push((x_cell.tail(), y_cell.tail())); - stack.push((x_cell.head(), y_cell.head())); - continue; - } - } - (_, _) => { - // cells don't match atoms - return false; - } - } - } else { - return unsafe { x.raw_equals(y) }; - } - } - - true -} - unsafe fn senior_pointer_first( stack: &NockStack, a: *const u64, @@ -1198,46 +1118,3 @@ impl Stack for NockStack { self.layout_alloc(layout) } } - -#[cfg(test)] -mod tests { - use super::*; - use crate::mem::NockStack; - use crate::noun::{Atom, Noun, D, T}; - use ibig::{ubig, UBig}; - - pub fn init_stack() -> NockStack { - NockStack::new(8 << 10 << 10, 0) - } - - #[allow(non_snake_case)] - pub fn A(stack: &mut NockStack, ubig: &UBig) -> Noun { - Atom::from_ubig(stack, ubig).as_noun() - } - - #[test] - fn test_non_unifying_equality() { - let s = &mut init_stack(); - let d1 = D(1); - let d2 = D(2); - let d3 = D(1); - let i1 = A(s, &ubig!(0xC0FFEEBABEB00B1E5)); - let i2 = A(s, &ubig!(0xBADDECAFC0FFEE000)); - let i3 = A(s, &ubig!(0xC0FFEEBABEB00B1E5)); - let c1 = T(s, &[d1, i3]); - let c2 = T(s, &[d2, i2]); - let c3 = T(s, &[d3, i1]); - - assert!(non_unifying_equality(d1, d1)); - assert!(!non_unifying_equality(d1, d2)); - assert!(non_unifying_equality(d1, d3)); - - assert!(non_unifying_equality(i1, i1)); - assert!(!non_unifying_equality(i1, i2)); - assert!(non_unifying_equality(i1, i3)); - - assert!(non_unifying_equality(c1, c1)); - assert!(!non_unifying_equality(c1, c2)); - assert!(non_unifying_equality(c1, c3)); - } -} diff --git a/rust/ares/src/noun.rs b/rust/ares/src/noun.rs index c9a372a..232d391 100644 --- a/rust/ares/src/noun.rs +++ b/rust/ares/src/noun.rs @@ -1,4 +1,4 @@ -use crate::mem::{non_unifying_equality, word_size_of, NockStack}; +use crate::mem::{word_size_of, NockStack}; use bitvec::prelude::{BitSlice, Lsb0}; use either::{Either, Left, Right}; use ibig::{Stack, UBig}; @@ -1146,12 +1146,6 @@ impl fmt::Display for Noun { } } -impl std::cmp::PartialEq for Noun { - fn eq(&self, other: &Noun) -> bool { - non_unifying_equality(*self, *other) - } -} - impl Slots for Noun {} impl private::RawSlots for Noun { fn raw_slot(&self, axis: &BitSlice) -> Result {