Remove PartialEq and Eq from AmlValue

It is not correct for AmlValue to be PartialEq or Eq, as to correctly compare
two AML objects, various conversion rules need to be applied, if they are
appropriate. This requires the context, and so is implemented with a custom
`cmp` method.

This is why we can't hold `AmlValue`s in `AmlError`, requiring the change
to use `Propagate`.

Unfortunately, this basically breaks our testing framework. I've made the
changes needed to fix all of this, but it's a bit nasty, and would probably
benefit from some cleaning up in the future.

aml/src/namespace.rs

@@ -543,6 +543,7 @@ impl NameComponent {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test_utils::crudely_cmp_values;
 
     #[test]
     fn test_aml_name_from_str() {
@@ -677,16 +678,22 @@ mod tests {
         /*
          * Get objects using their absolute paths.
          */
-        assert_eq!(namespace.get_by_path(&AmlName::from_str("\\MOO").unwrap()), Ok(&AmlValue::Boolean(true)));
-        assert_eq!(
-            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.A").unwrap()),
-            Ok(&AmlValue::Integer(12345))
-        );
-        assert_eq!(namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.B").unwrap()), Ok(&AmlValue::Integer(6)));
-        assert_eq!(
-            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.C").unwrap()),
-            Ok(&AmlValue::String(String::from("hello, world!")))
-        );
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\MOO").unwrap()).unwrap(),
+            &AmlValue::Boolean(true)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.A").unwrap()).unwrap(),
+            &AmlValue::Integer(12345)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.B").unwrap()).unwrap(),
+            &AmlValue::Integer(6)
+        ));
+        assert!(crudely_cmp_values(
+            namespace.get_by_path(&AmlName::from_str("\\FOO.BAR.C").unwrap()).unwrap(),
+            &AmlValue::String(String::from("hello, world!"))
+        ));
 
         /*
          * Search for some objects that should use search rules.

aml/src/term_object.rs

@@ -612,40 +612,48 @@ mod test {
     #[test]
     fn test_computational_data() {
         let mut context = make_test_context();
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x00, 0x34, 0x12], &mut context),
             AmlValue::Integer(0),
             &[0x34, 0x12]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x01, 0x18, 0xf3], &mut context),
             AmlValue::Integer(1),
             &[0x18, 0xf3]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0xff, 0x98, 0xc3], &mut context),
             AmlValue::Integer(u64::max_value()),
             &[0x98, 0xc3]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x5b, 0x30], &mut context),
             AmlValue::Integer(crate::AML_INTERPRETER_REVISION),
             &[]
         );
-        check_ok!(computational_data().parse(&[0x0a, 0xf3, 0x35], &mut context), AmlValue::Integer(0xf3), &[0x35]);
-        check_ok!(computational_data().parse(&[0x0b, 0xf3, 0x35], &mut context), AmlValue::Integer(0x35f3), &[]);
-        check_ok!(
+        check_ok_value!(
+            computational_data().parse(&[0x0a, 0xf3, 0x35], &mut context),
+            AmlValue::Integer(0xf3),
+            &[0x35]
+        );
+        check_ok_value!(
+            computational_data().parse(&[0x0b, 0xf3, 0x35], &mut context),
+            AmlValue::Integer(0x35f3),
+            &[]
+        );
+        check_ok_value!(
             computational_data().parse(&[0x0c, 0xf3, 0x35, 0x12, 0x65, 0xff, 0x00], &mut context),
             AmlValue::Integer(0x651235f3),
             &[0xff, 0x00]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data()
                 .parse(&[0x0e, 0xf3, 0x35, 0x12, 0x65, 0xff, 0x00, 0x67, 0xde, 0x28], &mut context),
             AmlValue::Integer(0xde6700ff651235f3),
             &[0x28]
         );
-        check_ok!(
+        check_ok_value!(
             computational_data().parse(&[0x0d, b'A', b'B', b'C', b'D', b'\0', 0xff, 0xf5], &mut context),
             AmlValue::String(String::from("ABCD")),
             &[0xff, 0xf5]

aml/src/test_utils.rs

@@ -1,4 +1,4 @@
-use crate::{AmlContext, Handler};
+use crate::{parser::Propagate, AmlContext, AmlValue, Handler};
 use alloc::boxed::Box;
 
 struct TestHandler;
@@ -71,14 +71,16 @@ impl Handler for TestHandler {
 }
 
 pub(crate) fn make_test_context() -> AmlContext {
-    AmlContext::new(Box::new(TestHandler), false, crate::DebugVerbosity::None)
+    AmlContext::new(Box::new(TestHandler), crate::DebugVerbosity::None)
 }
 
 pub(crate) macro check_err($parse: expr, $error: pat, $remains: expr) {
     match $parse {
         Ok((remains, _, result)) => panic!("Expected Err, got {:#?}. Remaining = {:#x?}", result, remains),
-        Err((remains, _, $error)) if *remains == *$remains => (),
-        Err((remains, _, $error)) => panic!("Correct error, incorrect stream returned: {:#x?}", remains),
+        Err((remains, _, Propagate::Err($error))) if *remains == *$remains => (),
+        Err((remains, _, Propagate::Err($error))) => {
+            panic!("Correct error, incorrect stream returned: {:#x?}", remains)
+        }
         Err((_, _, err)) => panic!("Got wrong error: {:?}", err),
     }
 }
@@ -93,3 +95,86 @@ pub(crate) macro check_ok($parse: expr, $expected: expr, $remains: expr) {
         Err((_, _, err)) => panic!("Expected Ok, got {:#?}", err),
     }
 }
+
+pub(crate) macro check_ok_value($parse: expr, $expected: expr, $remains: expr) {
+    match $parse {
+        Ok((remains, _, ref result)) if remains == *$remains && crudely_cmp_values(result, &$expected) => (),
+        Ok((remains, _, ref result)) if crudely_cmp_values(result, &$expected) => {
+            panic!("Correct result, incorrect slice returned: {:x?}", remains)
+        }
+        Ok((_, _, ref result)) => panic!("Successfully parsed Ok, but it was wrong: {:#?}", result),
+        Err((_, _, err)) => panic!("Expected Ok, got {:#?}", err),
+    }
+}
+
+/// This is a bad (but good for testing) way of comparing `AmlValue`s, which tests that they're exactly the same if
+/// it can, and gives up if it can't. It's useful in tests to be able to see if you're getting the `AmlValue` that
+/// you're expecting.
+///
+/// NOTE: almost all of the `AmlValue` variants are `Eq`, and so for a long time, `AmlValue` implemented `Eq`.
+/// However, this is a footgun as, in the real interpreter, you rarely want to directly compare values, as you need
+/// to apply the AML value conversion rules to compare them correctly. This is therefore only useful for artificial
+/// testing scenarios.
+pub(crate) fn crudely_cmp_values(a: &AmlValue, b: &AmlValue) -> bool {
+    match a {
+        AmlValue::Boolean(a) => match b {
+            AmlValue::Boolean(b) => a == b,
+            _ => false,
+        },
+        AmlValue::Integer(a) => match b {
+            AmlValue::Integer(b) => a == b,
+            _ => false,
+        },
+        AmlValue::String(ref a) => match b {
+            AmlValue::String(ref b) => a == b,
+            _ => false,
+        },
+        AmlValue::OpRegion { region, offset, length, parent_device } => match b {
+            AmlValue::OpRegion {
+                region: b_region,
+                offset: b_offset,
+                length: b_length,
+                parent_device: b_parent_device,
+            } => {
+                region == b_region && offset == b_offset && length == b_length && parent_device == b_parent_device
+            }
+            _ => false,
+        },
+        AmlValue::Field { region, flags, offset, length } => match b {
+            AmlValue::Field { region: b_region, flags: b_flags, offset: b_offset, length: b_length } => {
+                region == b_region && flags == b_flags && offset == b_offset && length == b_length
+            }
+            _ => false,
+        },
+        AmlValue::Method { flags, code } => match b {
+            AmlValue::Method { flags: b_flags, code: b_code } => flags == b_flags && code == b_code,
+            _ => false,
+        },
+        AmlValue::Buffer(a) => match b {
+            AmlValue::Buffer(b) => a == b,
+            _ => false,
+        },
+        AmlValue::Processor { id, pblk_address, pblk_len } => match b {
+            AmlValue::Processor { id: b_id, pblk_address: b_pblk_address, pblk_len: b_pblk_len } => {
+                id == b_id && pblk_address == b_pblk_address && pblk_len == b_pblk_len
+            }
+            _ => false,
+        },
+        AmlValue::Mutex { sync_level } => match b {
+            AmlValue::Mutex { sync_level: b_sync_level } => sync_level == b_sync_level,
+            _ => false,
+        },
+        AmlValue::Package(a) => match b {
+            AmlValue::Package(b) => {
+                for (a, b) in a.iter().zip(b) {
+                    if crudely_cmp_values(a, b) == false {
+                        return false;
+                    }
+                }
+
+                true
+            }
+            _ => false,
+        },
+    }
+}

aml/src/value.rs

@@ -140,7 +140,7 @@ pub enum AmlType {
     ThermalZone,
 }
 
-#[derive(Clone, PartialEq, Eq, Debug)]
+#[derive(Clone, Debug)]
 pub enum AmlValue {
     Boolean(bool),
     Integer(u64),