Finally properly handle error propagation in or and choice!

This was surprisingly easy to get working in the new parser, and fixes the
bad diagnostics we were previously seeing, as well as enabling us to use
the error system to propagate return values of control methods. It also
allows us to fix some bad unit tests that had to accept the "wrong" error
being produced due to use of `choice!`, which should appear as an
implementation detail but wasn't.

aml_parser/src/lib.rs

@@ -76,9 +76,10 @@ pub enum AmlError {
     UnterminatedStringConstant,
     InvalidStringConstant,
     InvalidRegionSpace(u8),
-    /// Error produced when none of the parsers in a `choice!` could parse the next part of the
-    /// stream.
-    NoParsersCouldParse,
+    /// Emitted by a parser when it's clear that the stream doesn't encode the object parsed by
+    /// that parser (e.g. the wrong opcode starts the stream). This is handled specially by some
+    /// parsers such as `or` and `choice!`.
+    WrongParser,
 
     /*
      * Errors produced querying the namespace.

aml_parser/src/name_object.rs

@@ -270,12 +270,12 @@ mod tests {
     fn test_name_path() {
         let mut context = AmlContext::new();
 
-        check_err!(name_path().parse(&[], &mut context), AmlError::NoParsersCouldParse, &[]);
+        check_err!(name_path().parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         check_ok!(name_path().parse(&[0x00], &mut context), alloc::vec![], &[]);
         check_ok!(name_path().parse(&[0x00, 0x00], &mut context), alloc::vec![], &[0x00]);
         check_err!(
             name_path().parse(&[0x2e, b'A'], &mut context),
-            AmlError::NoParsersCouldParse,
+            AmlError::UnexpectedEndOfStream,
             &[0x2e, b'A']
         );
         check_ok!(

aml_parser/src/opcode.rs

@@ -52,7 +52,7 @@ where
     move |input: &'a [u8], context: &'c mut AmlContext| match input.first() {
         None => Err((input, context, AmlError::UnexpectedEndOfStream)),
         Some(&byte) if byte == opcode => Ok((&input[1..], context, ())),
-        Some(&byte) => Err((input, context, AmlError::UnexpectedByte(byte))),
+        Some(_) => Err((input, context, AmlError::WrongParser)),
     }
 }
 
@@ -95,7 +95,7 @@ mod tests {
         let mut context = AmlContext::new();
         check_err!(
             ext_opcode(EXT_DEF_FIELD_OP).parse(&[EXT_DEF_FIELD_OP, EXT_DEF_FIELD_OP], &mut context),
-            AmlError::UnexpectedByte(EXT_DEF_FIELD_OP),
+            AmlError::WrongParser,
             &[EXT_DEF_FIELD_OP, EXT_DEF_FIELD_OP]
         );
         check_ok!(

aml_parser/src/parser.rs

@@ -29,9 +29,10 @@ where
         DiscardResult { parser: self, _phantom: PhantomData }
     }
 
-    /// Try parsing with `self`. If it fails, try parsing with `other`, returning the result of the
-    /// first of the two parsers to succeed. To `or` multiple parsers ergonomically, see the
-    /// `choice!` macro.
+    /// Try parsing with `self`. If it succeeds, return its result. If it returns `AmlError::WrongParser`, try
+    /// parsing with `other`, returning the result of that parser in all cases. Other errors from the first
+    /// parser are propagated without attempting the second parser. To chain more than two parsers using
+    /// `or`, see the `choice!` macro.
     fn or<OtherParser>(self, other: OtherParser) -> Or<'a, 'c, Self, OtherParser, R>
     where
         OtherParser: Parser<'a, 'c, R>,
@@ -255,12 +256,11 @@ where
     P2: Parser<'a, 'c, R>,
 {
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, R> {
-        let context = match self.p1.parse(input, context) {
-            Ok(parse_result) => return Ok(parse_result),
-            Err((_, context, _)) => context,
-        };
-
-        self.p2.parse(input, context)
+        match self.p1.parse(input, context) {
+            Ok(parse_result) => Ok(parse_result),
+            Err((_, context, AmlError::WrongParser)) => self.p2.parse(input, context),
+            Err((_, context, err)) => Err((input, context, err)),
+        }
     }
 }
 
@@ -393,23 +393,25 @@ where
     }
 }
 
-pub(crate) fn emit_no_parsers_could_parse<'a, 'c, R>() -> impl Parser<'a, 'c, R>
+/// This is a helper parser used in the `choice` macro to emit `AmlError::WrongParser`
+/// unconditionally. It should not be used directly.
+pub(crate) fn no_parsers_could_parse<'a, 'c, R>() -> impl Parser<'a, 'c, R>
 where
     'c: 'a,
 {
-    |input: &'a [u8], context| Err((input, context, AmlError::NoParsersCouldParse))
+    |input: &'a [u8], context| Err((input, context, AmlError::WrongParser))
 }
 
 /// Takes a number of parsers, and tries to apply each one to the input in order. Returns the
 /// result of the first one that succeeds, or fails if all of them fail.
 pub(crate) macro choice {
     () => {
-        emit_no_parsers_could_parse()
+        no_parsers_could_parse()
     },
 
     ($first_parser: expr) => {
         $first_parser
-        .or(emit_no_parsers_could_parse())
+        .or(no_parsers_could_parse())
     },
 
     ($first_parser: expr, $($other_parser: expr),*) => {
@@ -417,7 +419,7 @@ pub(crate) macro choice {
         $(
             .or($other_parser)
          )*
-        .or(emit_no_parsers_could_parse())
+        .or(no_parsers_could_parse())
     }
 }
 

aml_parser/src/term_object.rs

@@ -547,7 +547,7 @@ where
             opcode::ONE_OP => Ok((new_input, context, AmlValue::Integer(1))),
             opcode::ONES_OP => Ok((new_input, context, AmlValue::Integer(u64::max_value()))),
 
-            _ => Err((input, context, AmlError::UnexpectedByte(op))),
+            _ => Err((input, context, AmlError::WrongParser)),
         }
     };