Merge #47

47: [WIP] Make AML parser useful r=IsaacWoods a=IsaacWoods

Also includes some quick fixes.

Now that we can parse a sufficient subset of AML to parse a standard set of tables (QEMU's q35 DSDT and SSDTs), we can flesh out the parts of the parser that actually provide useful information to the OS.

Todo:
- [x] Provide methods to lookup objects in the namespace
- [x] Framework for executing control methods
- [x] Parse `DefLEqual`
- [x] Start of parsing `Type1Opcode` and `Type2Opcode`
- [x] Start of parsing `MethodInvocation`
- [x] Parse `DefIfElse`
- [x] Parse `DefReturn`
- [x] Implement namespace search rules for single-segment paths
- [x] Parse `SimpleName`
- [x] Parse `DefStore`
- [x] Parse `DebugObj`
- [x] Parse args and locals

Co-authored-by: Isaac Woods <isaacwoods.home@gmail.com>

acpi/src/fadt.rs

@@ -1,12 +1,4 @@
-use crate::{
-    sdt::SdtHeader,
-    Acpi,
-    AcpiError,
-    AcpiHandler,
-    AmlTable,
-    GenericAddress,
-    PhysicalMapping,
-};
+use crate::{sdt::SdtHeader, Acpi, AcpiError, AcpiHandler, AmlTable, GenericAddress, PhysicalMapping};
 
 type ExtendedField<T> = crate::sdt::ExtendedField<T, typenum::U2>;
 

acpi/src/handler.rs

@@ -27,11 +27,7 @@ pub trait AcpiHandler {
     /// page-aligned, so the implementation may have to add padding to either end. The given
     /// size must be greater or equal to the size of a `T`. The virtual address the memory is
     /// mapped to does not matter, as long as it is accessible from `acpi`.
-    fn map_physical_region<T>(
-        &mut self,
-        physical_address: usize,
-        size: usize,
-    ) -> PhysicalMapping<T>;
+    fn map_physical_region<T>(&mut self, physical_address: usize, size: usize) -> PhysicalMapping<T>;
 
     /// Unmap the given physical mapping. Safe because we consume the mapping, and so it can't be
     /// used after being passed to this function.

acpi/src/hpet.rs

@@ -33,10 +33,7 @@ pub(crate) struct HpetTable {
     page_protection_oem: u8,
 }
 
-pub(crate) fn parse_hpet(
-    acpi: &mut Acpi,
-    mapping: &PhysicalMapping<HpetTable>,
-) -> Result<(), AcpiError> {
+pub(crate) fn parse_hpet(acpi: &mut Acpi, mapping: &PhysicalMapping<HpetTable>) -> Result<(), AcpiError> {
     (*mapping).header.validate(b"HPET")?;
     let hpet = &*mapping;
 

acpi/src/lib.rs

@@ -164,10 +164,7 @@ where
     parse_validated_rsdp(handler, rsdp_mapping)
 }
 
-fn parse_validated_rsdp<H>(
-    handler: &mut H,
-    rsdp_mapping: PhysicalMapping<Rsdp>,
-) -> Result<Acpi, AcpiError>
+fn parse_validated_rsdp<H>(handler: &mut H, rsdp_mapping: PhysicalMapping<Rsdp>) -> Result<Acpi, AcpiError>
 where
     H: AcpiHandler,
 {
@@ -197,11 +194,7 @@ where
 ///
 /// If the given revision is 0, an address to the RSDT is expected. Otherwise, an address to
 /// the XSDT is expected.
-pub fn parse_rsdt<H>(
-    handler: &mut H,
-    revision: u8,
-    physical_address: usize,
-) -> Result<Acpi, AcpiError>
+pub fn parse_rsdt<H>(handler: &mut H, revision: u8, physical_address: usize) -> Result<Acpi, AcpiError>
 where
     H: AcpiHandler,
 {
@@ -217,8 +210,7 @@ where
     };
 
     let header = sdt::peek_at_sdt_header(handler, physical_address);
-    let mapping =
-        handler.map_physical_region::<SdtHeader>(physical_address, header.length() as usize);
+    let mapping = handler.map_physical_region::<SdtHeader>(physical_address, header.length() as usize);
 
     if revision == 0 {
         /*
@@ -226,14 +218,12 @@ where
          */
         (*mapping).validate(b"RSDT")?;
 
-        let num_tables =
-            ((*mapping).length() as usize - mem::size_of::<SdtHeader>()) / mem::size_of::<u32>();
+        let num_tables = ((*mapping).length() as usize - mem::size_of::<SdtHeader>()) / mem::size_of::<u32>();
         let tables_base =
             ((mapping.virtual_start.as_ptr() as usize) + mem::size_of::<SdtHeader>()) as *const u32;
 
         for i in 0..num_tables {
-            sdt::dispatch_sdt(&mut acpi, handler, unsafe { *tables_base.offset(i as isize) }
-                as usize)?;
+            sdt::dispatch_sdt(&mut acpi, handler, unsafe { *tables_base.offset(i as isize) } as usize)?;
         }
     } else {
         /*
@@ -241,14 +231,12 @@ where
          */
         (*mapping).validate(b"XSDT")?;
 
-        let num_tables =
-            ((*mapping).length() as usize - mem::size_of::<SdtHeader>()) / mem::size_of::<u64>();
+        let num_tables = ((*mapping).length() as usize - mem::size_of::<SdtHeader>()) / mem::size_of::<u64>();
         let tables_base =
             ((mapping.virtual_start.as_ptr() as usize) + mem::size_of::<SdtHeader>()) as *const u64;
 
         for i in 0..num_tables {
-            sdt::dispatch_sdt(&mut acpi, handler, unsafe { *tables_base.offset(i as isize) }
-                as usize)?;
+            sdt::dispatch_sdt(&mut acpi, handler, unsafe { *tables_base.offset(i as isize) } as usize)?;
         }
     }
 

acpi/src/madt.rs

@@ -1,12 +1,5 @@
 use crate::{
-    interrupt::{
-        InterruptModel,
-        InterruptSourceOverride,
-        IoApic,
-        NmiSource,
-        Polarity,
-        TriggerMode,
-    },
+    interrupt::{InterruptModel, InterruptSourceOverride, IoApic, NmiSource, Polarity, TriggerMode},
     sdt::SdtHeader,
     Acpi,
     AcpiError,
@@ -47,9 +40,7 @@ pub(crate) struct Madt {
 impl Madt {
     fn entries(&self) -> MadtEntryIter {
         MadtEntryIter {
-            pointer: unsafe {
-                (self as *const Madt as *const u8).offset(mem::size_of::<Madt>() as isize)
-            },
+            pointer: unsafe { (self as *const Madt as *const u8).offset(mem::size_of::<Madt>() as isize) },
             remaining_length: self.header.length() - mem::size_of::<Madt>() as u32,
             _phantom: PhantomData,
         }
@@ -399,10 +390,7 @@ where
 
 /// This parses the MADT and gathers information about a APIC interrupt model. We error if we
 /// encounter an entry that doesn't configure the APIC.
-fn parse_apic_model(
-    acpi: &mut Acpi,
-    mapping: &PhysicalMapping<Madt>,
-) -> Result<InterruptModel, AcpiError> {
+fn parse_apic_model(acpi: &mut Acpi, mapping: &PhysicalMapping<Madt>) -> Result<InterruptModel, AcpiError> {
     use crate::interrupt::LocalInterruptLine;
 
     let mut local_apic_address = (*mapping).local_apic_address as u64;
@@ -451,9 +439,7 @@ fn parse_apic_model(
 
             MadtEntry::InterruptSourceOverride(ref entry) => {
                 if entry.bus != 0 {
-                    return Err(AcpiError::InvalidMadt(
-                        MadtError::InterruptOverrideEntryHasInvalidBus,
-                    ));
+                    return Err(AcpiError::InvalidMadt(MadtError::InterruptOverrideEntryHasInvalidBus));
                 }
 
                 let (polarity, trigger_mode) = parse_mps_inti_flags(entry.flags)?;

acpi/src/mcfg.rs

@@ -1,7 +1,6 @@
 use crate::{handler::PhysicalMapping, sdt::SdtHeader, Acpi, AcpiError};
 use alloc::vec::Vec;
 use core::{mem, slice};
-use log::info;
 
 /// Describes a set of regions of physical memory used to access the PCI-E configuration space. A
 /// region  is created for each entry in the MCFG. Given the segment group, bus, device number, and
@@ -16,13 +15,7 @@ pub struct PciConfigRegions {
 impl PciConfigRegions {
     /// Get the physical address of the start of the configuration space for a given PCI-E device
     /// function. Returns `None` if there isn't an entry in the MCFG that manages that device.
-    pub fn physical_address(
-        &self,
-        segment_group_no: u16,
-        bus: u8,
-        device: u8,
-        function: u8,
-    ) -> Option<u64> {
+    pub fn physical_address(&self, segment_group_no: u16, bus: u8, device: u8, function: u8) -> Option<u64> {
         // First, find the memory region that handles this segment and bus. This method is fine
         // because there should only be one region that handles each segment group + bus
         // combination.
@@ -50,12 +43,6 @@ pub(crate) struct Mcfg {
 impl Mcfg {
     fn entries(&self) -> &[McfgEntry] {
         let length = self.header.length() as usize - mem::size_of::<Mcfg>();
-        info!(
-            "(raw length = {}, header length = {}, length of entries = {})",
-            self.header.length(),
-            mem::size_of::<SdtHeader>(),
-            length
-        );
         assert!(length % mem::size_of::<McfgEntry>() == 0);
         let num_entries = length / mem::size_of::<McfgEntry>();
 
@@ -77,10 +64,7 @@ struct McfgEntry {
     _reserved: u32,
 }
 
-pub(crate) fn parse_mcfg(
-    acpi: &mut Acpi,
-    mapping: &PhysicalMapping<Mcfg>,
-) -> Result<(), AcpiError> {
+pub(crate) fn parse_mcfg(acpi: &mut Acpi, mapping: &PhysicalMapping<Mcfg>) -> Result<(), AcpiError> {
     (*mapping).header.validate(b"MCFG")?;
 
     acpi.pci_config_regions =

acpi/src/sdt.rs

@@ -1,13 +1,4 @@
-use crate::{
-    fadt::Fadt,
-    hpet::HpetTable,
-    madt::Madt,
-    mcfg::Mcfg,
-    Acpi,
-    AcpiError,
-    AcpiHandler,
-    AmlTable,
-};
+use crate::{fadt::Fadt, hpet::HpetTable, madt::Madt, mcfg::Mcfg, Acpi, AcpiError, AcpiHandler, AmlTable};
 use core::{marker::PhantomData, mem, str};
 use log::{trace, warn};
 use typenum::Unsigned;
@@ -156,8 +147,7 @@ pub(crate) fn peek_at_sdt_header<H>(handler: &mut H, physical_address: usize) ->
 where
     H: AcpiHandler,
 {
-    let mapping =
-        handler.map_physical_region::<SdtHeader>(physical_address, mem::size_of::<SdtHeader>());
+    let mapping = handler.map_physical_region::<SdtHeader>(physical_address, mem::size_of::<SdtHeader>());
     let header = (*mapping).clone();
     handler.unmap_physical_region(mapping);
 
@@ -175,11 +165,7 @@ where
     H: AcpiHandler,
 {
     let header = peek_at_sdt_header(handler, physical_address);
-    trace!(
-        "Found ACPI table with signature {:?} and length {:?}",
-        header.signature(),
-        header.length()
-    );
+    trace!("Found ACPI table with signature {:?} and length {:?}", header.signature(), header.length());
 
     /*
      * For a recognised signature, a new physical mapping should be created with the correct type
@@ -187,15 +173,14 @@ where
      */
     match header.signature() {
         "FACP" => {
-            let fadt_mapping =
-                handler.map_physical_region::<Fadt>(physical_address, mem::size_of::<Fadt>());
+            let fadt_mapping = handler.map_physical_region::<Fadt>(physical_address, mem::size_of::<Fadt>());
             crate::fadt::parse_fadt(acpi, handler, &fadt_mapping)?;
             handler.unmap_physical_region(fadt_mapping);
         }
 
         "HPET" => {
-            let hpet_mapping = handler
-                .map_physical_region::<HpetTable>(physical_address, mem::size_of::<HpetTable>());
+            let hpet_mapping =
+                handler.map_physical_region::<HpetTable>(physical_address, mem::size_of::<HpetTable>());
             crate::hpet::parse_hpet(acpi, &hpet_mapping)?;
             handler.unmap_physical_region(hpet_mapping);
         }

aml_parser/src/lib.rs

@@ -11,6 +11,11 @@
 //! unmap the memory the table was mapped into - all the information needed will be extracted and
 //! allocated on the heap.
 //!
+//! You can then access specific objects by name like so: e.g.
+//! ```ignore
+//! let my_aml_value = aml_context.lookup(&AmlName::from_str("\\_SB.PCI0.S08._ADR").unwrap());
+//! ```
+//!
 //! ### About the parser
 //! The parser is written using a set of custom parser combinators - the code can be confusing on
 //! first reading, but provides an extensible and type-safe way to write parsers. For an easy
@@ -37,49 +42,72 @@ extern crate std;
 #[cfg(test)]
 mod test_utils;
 
+pub(crate) mod misc;
 pub(crate) mod name_object;
+pub(crate) mod namespace;
 pub(crate) mod opcode;
 pub(crate) mod parser;
 pub(crate) mod pkg_length;
 pub(crate) mod term_object;
+pub(crate) mod type1;
+pub(crate) mod type2;
 pub mod value;
 
-pub use crate::value::AmlValue;
+pub use crate::{
+    namespace::{AmlHandle, AmlName, Namespace},
+    value::AmlValue,
+};
 
-use alloc::collections::BTreeMap;
+use alloc::string::String;
 use log::error;
-use name_object::AmlName;
+use misc::{ArgNum, LocalNum};
 use parser::Parser;
 use pkg_length::PkgLength;
+use term_object::term_list;
+use value::Args;
 
 /// AML has a `RevisionOp` operator that returns the "AML interpreter revision". It's not clear
 /// what this is actually used for, but this is ours.
 pub const AML_INTERPRETER_REVISION: u64 = 0;
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-pub enum AmlError {
-    UnexpectedEndOfStream,
-    UnexpectedByte(u8),
-    InvalidNameSeg([u8; 4]),
-    InvalidFieldFlags,
-    IncompatibleValueConversion,
-    UnterminatedStringConstant,
-    InvalidStringConstant,
-    InvalidRegionSpace(u8),
-    /// Error produced when none of the parsers in a `choice!` could parse the next part of the
-    /// stream.
-    NoParsersCouldParse,
-}
-
 #[derive(Debug)]
 pub struct AmlContext {
-    namespace: BTreeMap<AmlName, AmlValue>,
+    pub namespace: Namespace,
     current_scope: AmlName,
+
+    /*
+     * AML local variables. These are used when we invoke a control method. A `None` value
+     * represents a null AML object.
+     */
+    local_0: Option<AmlValue>,
+    local_1: Option<AmlValue>,
+    local_2: Option<AmlValue>,
+    local_3: Option<AmlValue>,
+    local_4: Option<AmlValue>,
+    local_5: Option<AmlValue>,
+    local_6: Option<AmlValue>,
+    local_7: Option<AmlValue>,
+
+    /// If we're currently invoking a control method, this stores the arguments that were passed to
+    /// it. It's `None` if we aren't invoking a method.
+    current_args: Option<Args>,
 }
 
 impl AmlContext {
     pub fn new() -> AmlContext {
-        AmlContext { namespace: BTreeMap::new(), current_scope: AmlName::root() }
+        AmlContext {
+            namespace: Namespace::new(),
+            current_scope: AmlName::root(),
+            local_0: None,
+            local_1: None,
+            local_2: None,
+            local_3: None,
+            local_4: None,
+            local_5: None,
+            local_6: None,
+            local_7: None,
+            current_args: None,
+        }
     }
 
     pub fn parse_table(&mut self, stream: &[u8]) -> Result<(), AmlError> {
@@ -90,33 +118,131 @@ impl AmlContext {
         let table_length = PkgLength::from_raw_length(stream, stream.len() as u32) as PkgLength;
         match term_object::term_list(table_length).parse(stream, self) {
             Ok(_) => Ok(()),
-            Err((remaining, _context, err)) => {
+            Err((_, _, err)) => {
                 error!("Failed to parse AML stream. Err = {:?}", err);
                 Err(err)
             }
         }
     }
 
-    /// Resolves a given path relative to the current scope (if the given path is not absolute).
-    /// The returned path can be used to index the namespace.
-    pub fn resolve_path(&mut self, path: &AmlName) -> AmlName {
-        // TODO: we should normalize the path by resolving prefix chars etc.
-
-        // If the path is absolute, just return it.
-        if path.is_absolute() {
-            return path.clone();
+    /// Invoke a method referred to by its path in the namespace, with the given arguments.
+    pub fn invoke_method(&mut self, path: &AmlName, args: Args) -> Result<AmlValue, AmlError> {
+        if let AmlValue::Method { flags, code } = self.namespace.get_by_path(path)?.clone() {
+            /*
+             * First, set up the state we expect to enter the method with, but clearing local
+             * variables to "null" and setting the arguments.
+             */
+            self.current_scope = path.clone();
+            self.current_args = Some(args);
+            self.local_0 = None;
+            self.local_1 = None;
+            self.local_2 = None;
+            self.local_3 = None;
+            self.local_4 = None;
+            self.local_5 = None;
+            self.local_6 = None;
+            self.local_7 = None;
+
+            let return_value =
+                match term_list(PkgLength::from_raw_length(&code, code.len() as u32)).parse(&code, self) {
+                    // If the method doesn't return a value, we implicitly return `0`
+                    Ok(_) => Ok(AmlValue::Integer(0)),
+                    Err((_, _, AmlError::Return(result))) => Ok(result),
+                    Err((_, _, err)) => {
+                        error!("Failed to execute control method: {:?}", err);
+                        Err(err)
+                    }
+                };
+
+            /*
+             * Now clear the state.
+             */
+            self.current_args = None;
+            self.local_0 = None;
+            self.local_1 = None;
+            self.local_2 = None;
+            self.local_3 = None;
+            self.local_4 = None;
+            self.local_5 = None;
+            self.local_6 = None;
+            self.local_7 = None;
+
+            return_value
+        } else {
+            Err(AmlError::IncompatibleValueConversion)
         }
+    }
 
-        // Otherwise, it's relative to the current scope so append it onto that.
-        let mut new_path = self.current_scope.clone();
-        new_path.0.extend_from_slice(&(path.0));
-        new_path
+    pub(crate) fn current_arg(&self, arg: ArgNum) -> Result<&AmlValue, AmlError> {
+        self.current_args.as_ref().ok_or(AmlError::InvalidArgumentAccess(0xff))?.arg(arg)
     }
 
-    /// Add an `AmlValue` to the namespace. `path` can either be absolute, or relative (in which
-    /// case it's treated as relative to the current scope).
-    pub fn add_to_namespace(&mut self, path: AmlName, value: AmlValue) {
-        let resolved_path = self.resolve_path(&path);
-        self.namespace.insert(resolved_path, value);
+    /// Get the current value of a local by its local number.
+    ///
+    /// ### Panics
+    /// Panics if an invalid local number is passed (valid local numbers are `0..=7`)
+    pub(crate) fn local(&self, local: LocalNum) -> Result<&AmlValue, AmlError> {
+        match local {
+            0 => self.local_0.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            1 => self.local_1.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            2 => self.local_2.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            3 => self.local_3.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            4 => self.local_4.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            5 => self.local_5.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            6 => self.local_6.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            7 => self.local_7.as_ref().ok_or(AmlError::InvalidLocalAccess(local)),
+            _ => panic!("Invalid local number: {}", local),
+        }
     }
 }
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum AmlError {
+    /*
+     * Errors produced parsing the AML stream.
+     */
+    UnexpectedEndOfStream,
+    UnexpectedByte(u8),
+    InvalidNameSeg([u8; 4]),
+    InvalidFieldFlags,
+    IncompatibleValueConversion,
+    UnterminatedStringConstant,
+    InvalidStringConstant,
+    InvalidRegionSpace(u8),
+    /// 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 manipulating AML names.
+     */
+    /// Produced when trying to normalize a path that does not point to a valid level of the
+    /// namespace. E.g. `\_SB.^^PCI0` goes above the root of the namespace.
+    InvalidNormalizedName(String),
+    RootHasNoParent,
+
+    /*
+     * Errors produced working with the namespace.
+     */
+    /// Produced when a path is given that does not point to an object in the AML namespace.
+    ObjectDoesNotExist(String),
+    HandleDoesNotExist(AmlHandle),
+    /// Produced when two values with the same name are added to the namespace.
+    NameCollision(AmlName),
+
+    /*
+     * Errors produced executing control methods.
+     */
+    /// Produced when a method accesses an argument it does not have (e.g. a method that takes 2
+    /// arguments accesses `Arg4`). The inner value is the number of the argument accessed. If any
+    /// arguments are accessed when a method is not being executed, this error is produced with an
+    /// argument number of `0xff`.
+    InvalidArgumentAccess(ArgNum),
+    InvalidLocalAccess(LocalNum),
+    /// This is not a real error, but is used to propagate return values from within the deep
+    /// parsing call-stack. It should only be emitted when parsing a `DefReturn`. We use the
+    /// error system here because the way errors are propagated matches how we want to handle
+    /// return values.
+    Return(AmlValue),
+}

aml_parser/src/misc.rs

@@ -0,0 +1,80 @@
+use crate::{
+    opcode::{self, ext_opcode, opcode},
+    parser::{choice, comment_scope_verbose, id, Parser},
+};
+
+pub fn debug_obj<'a, 'c>() -> impl Parser<'a, 'c, ()>
+where
+    'c: 'a,
+{
+    /*
+     * DebugObj := ExtOpPrefix 0x31
+     */
+    ext_opcode(opcode::EXT_DEBUG_OP)
+}
+
+/// Takes a value between `0` and `7`, where 0 represents `Local0` etc.
+pub type LocalNum = u8;
+
+pub fn local_obj<'a, 'c>() -> impl Parser<'a, 'c, LocalNum>
+where
+    'c: 'a,
+{
+    /*
+     * LocalObj := Local0Op | Local1Op | Local2Op | Local3Op | Local4Op | Local5Op | Local6Op | Local7Op
+     * Local0Op := 0x60
+     * Local1Op := 0x61
+     * Local2Op := 0x62
+     * Local3Op := 0x63
+     * Local4Op := 0x64
+     * Local5Op := 0x65
+     * Local6Op := 0x66
+     * Local7Op := 0x67
+     */
+    let local_parser = |i, local_opcode| {
+        opcode(local_opcode).then(comment_scope_verbose("LocalObj", id())).map(move |((), _)| Ok(i))
+    };
+
+    choice!(
+        local_parser(0, opcode::LOCAL0_OP),
+        local_parser(1, opcode::LOCAL1_OP),
+        local_parser(2, opcode::LOCAL2_OP),
+        local_parser(3, opcode::LOCAL3_OP),
+        local_parser(4, opcode::LOCAL4_OP),
+        local_parser(5, opcode::LOCAL5_OP),
+        local_parser(6, opcode::LOCAL6_OP),
+        local_parser(7, opcode::LOCAL7_OP)
+    )
+}
+
+/// Takes a value between `0` and `6`, where 0 represents `Arg0` etc.
+pub type ArgNum = u8;
+
+pub fn arg_obj<'a, 'c>() -> impl Parser<'a, 'c, ArgNum>
+where
+    'c: 'a,
+{
+    /*
+     * ArgObj := Arg0Op | Arg1Op | Arg2Op | Arg3Op | Arg4Op | Arg5Op | Arg6Op
+     * Arg0Op = 0x68
+     * Arg1Op = 0x69
+     * Arg2Op = 0x6a
+     * Arg3Op = 0x6b
+     * Arg4Op = 0x6c
+     * Arg5Op = 0x6d
+     * Arg6Op = 0x6e
+     */
+    let arg_parser = |i, arg_opcode| {
+        opcode(arg_opcode).then(comment_scope_verbose("ArgObj", id())).map(move |((), _)| Ok(i))
+    };
+
+    choice!(
+        arg_parser(0, opcode::ARG0_OP),
+        arg_parser(1, opcode::ARG1_OP),
+        arg_parser(2, opcode::ARG2_OP),
+        arg_parser(3, opcode::ARG3_OP),
+        arg_parser(4, opcode::ARG4_OP),
+        arg_parser(5, opcode::ARG5_OP),
+        arg_parser(6, opcode::ARG6_OP)
+    )
+}

aml_parser/src/name_object.rs

@@ -1,55 +1,49 @@
 use crate::{
+    misc::{arg_obj, debug_obj, local_obj, ArgNum, LocalNum},
+    namespace::{AmlName, NameComponent},
     opcode::{opcode, DUAL_NAME_PREFIX, MULTI_NAME_PREFIX, NULL_NAME, PREFIX_CHAR, ROOT_CHAR},
     parser::{choice, comment_scope_verbose, consume, n_of, take, Parser},
     AmlContext,
     AmlError,
 };
-use alloc::{
-    string::{String, ToString},
-    vec::Vec,
-};
+use alloc::vec::Vec;
 use core::{fmt, str};
 
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
-pub struct AmlName(pub(crate) Vec<NameComponent>);
-
-impl AmlName {
-    pub fn root() -> AmlName {
-        AmlName(alloc::vec![NameComponent::Root])
-    }
-
-    pub fn from_name_seg(seg: NameSeg) -> AmlName {
-        AmlName(alloc::vec![NameComponent::Segment(seg)])
-    }
-
-    pub fn as_string(&self) -> String {
-        self.0
-            .iter()
-            .fold(String::new(), |name, component| match component {
-                NameComponent::Root => name + "\\",
-                NameComponent::Prefix => name + "^",
-                NameComponent::Segment(seg) => name + seg.as_str() + ".",
-            })
-            .trim_end_matches('.')
-            .to_string()
-    }
-
-    pub fn is_absolute(&self) -> bool {
-        self.0.first() == Some(&NameComponent::Root)
-    }
+/// Produced by the `Target`, `SimpleName`, and `SuperName` parsers
+#[derive(Clone, Debug)]
+pub enum Target {
+    Name(AmlName),
+    Debug,
+    Arg(ArgNum),
+    Local(LocalNum),
 }
 
-impl fmt::Display for AmlName {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.as_string())
-    }
+pub fn super_name<'a, 'c>() -> impl Parser<'a, 'c, Target>
+where
+    'c: 'a,
+{
+    /*
+     * SuperName := SimpleName | DebugObj | Type6Opcode
+     * TODO: this doesn't cover Type6Opcode yet
+     */
+    comment_scope_verbose("SuperName", choice!(debug_obj().map(|()| Ok(Target::Debug)), simple_name()))
 }
 
-#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
-pub enum NameComponent {
-    Root,
-    Prefix,
-    Segment(NameSeg),
+pub fn simple_name<'a, 'c>() -> impl Parser<'a, 'c, Target>
+where
+    'c: 'a,
+{
+    /*
+     * SimpleName := NameString | ArgObj | LocalObj
+     */
+    comment_scope_verbose(
+        "SimpleName",
+        choice!(
+            name_string().map(move |name| Ok(Target::Name(name))),
+            arg_obj().map(|arg_num| Ok(Target::Arg(arg_num))),
+            local_obj().map(|local_num| Ok(Target::Local(local_num)))
+        ),
+    )
 }
 
 pub fn name_string<'a, 'c>() -> impl Parser<'a, 'c, AmlName>
@@ -132,11 +126,9 @@ where
         let (new_input, context, ((), seg_count)) =
             opcode(MULTI_NAME_PREFIX).then(take()).parse(input, context)?;
         match n_of(name_seg(), usize::from(seg_count)).parse(new_input, context) {
-            Ok((new_input, context, name_segs)) => Ok((
-                new_input,
-                context,
-                name_segs.iter().map(|&seg| NameComponent::Segment(seg)).collect(),
-            )),
+            Ok((new_input, context, name_segs)) => {
+                Ok((new_input, context, name_segs.iter().map(|&seg| NameComponent::Segment(seg)).collect()))
+            }
             // Correct returned input to the one we haven't touched
             Err((_, context, err)) => Err((input, context, err)),
         }
@@ -144,9 +136,36 @@ where
 }
 
 #[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
-pub struct NameSeg([u8; 4]);
+pub struct NameSeg(pub(crate) [u8; 4]);
 
 impl NameSeg {
+    pub(crate) fn from_str(string: &str) -> Option<NameSeg> {
+        // Each NameSeg can only have four chars, and must have at least one
+        if string.len() < 1 || string.len() > 4 {
+            return None;
+        }
+
+        // We pre-fill the array with '_', so it will already be correct if the length is < 4
+        let mut seg = [b'_'; 4];
+        let bytes = string.as_bytes();
+
+        // Manually do the first one, because we have to check it's a LeadNameChar
+        if !is_lead_name_char(bytes[0]) {
+            return None;
+        }
+        seg[0] = bytes[0];
+
+        // Copy the rest of the chars, checking that they're NameChars
+        for i in 1..bytes.len() {
+            if !is_name_char(bytes[i]) {
+                return None;
+            }
+            seg[i] = bytes[i];
+        }
+
+        Some(NameSeg(seg))
+    }
+
     /// Turn a `NameSeg` into a `&str`. Returns it in a `ParseResult` so it's easy to use from
     /// inside parsers.
     pub fn as_str(&self) -> &str {
@@ -225,17 +244,16 @@ 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!(
-            name_path()
-                .parse(&[0x2e, b'A', b'B', b'C', b'D', b'E', b'_', b'F', b'G'], &mut context),
+            name_path().parse(&[0x2e, b'A', b'B', b'C', b'D', b'E', b'_', b'F', b'G'], &mut context),
             alloc::vec![
                 NameComponent::Segment(NameSeg([b'A', b'B', b'C', b'D'])),
                 NameComponent::Segment(NameSeg([b'E', b'_', b'F', b'G']))

aml_parser/src/namespace.rs

@@ -0,0 +1,347 @@
+use crate::{name_object::NameSeg, value::AmlValue, AmlError};
+use alloc::{
+    collections::BTreeMap,
+    string::{String, ToString},
+    vec::Vec,
+};
+use core::fmt;
+
+/// A handle is used to refer to an AML value without actually borrowing it until you need to
+/// access it (this makes borrowing situation much easier as you only have to consider who's
+/// borrowing the namespace). They can also be cached to avoid expensive namespace lookups.
+///
+/// Handles are never reused (the handle to a removed object will never be reused to point to a new
+/// object). This ensures handles cached by the library consumer will never point to an object they
+/// did not originally point to, but also means that, in theory, we can run out of handles on a
+/// very-long-running system (we are yet to see if this is a problem, practically).
+#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub struct AmlHandle(u32);
+
+impl AmlHandle {
+    pub(self) fn increment(&mut self) {
+        self.0 += 1;
+    }
+}
+
+pub struct Namespace {
+    /// This is a running count of ids, which are never reused. This is incremented every time we
+    /// add a new object to the namespace. We can then remove objects, freeing their memory, without
+    /// risking using the same id for two objects.
+    next_handle: AmlHandle,
+
+    /// This maps handles to actual values, and is used to access the actual AML values.
+    object_map: BTreeMap<AmlHandle, AmlValue>,
+
+    /// This maps names to handles, and should be used when you don't already have the handle to a
+    /// value.
+    // XXX: in the future, this could be replaced with a better data structure that doesn't store
+    // the entire name for each id. Instead, it would be a tree-like structure that stores each
+    // name segment, with a list of objects and their names, and a list of child scopes at that
+    // level.
+    name_map: BTreeMap<AmlName, AmlHandle>,
+}
+
+impl Namespace {
+    pub fn new() -> Namespace {
+        Namespace { next_handle: AmlHandle(0), object_map: BTreeMap::new(), name_map: BTreeMap::new() }
+    }
+
+    /// Add a value to the namespace at the given path, which must be a normalized, absolute AML
+    /// name. If you want to add at a path relative to a given scope, use `add_at_resolved_path`
+    /// instead.
+    pub fn add(&mut self, path: AmlName, value: AmlValue) -> Result<AmlHandle, AmlError> {
+        assert!(path.is_absolute());
+        assert!(path.is_normal());
+
+        if self.name_map.contains_key(&path) {
+            return Err(AmlError::NameCollision(path.clone()));
+        }
+
+        let handle = self.next_handle;
+        self.next_handle.increment();
+
+        self.object_map.insert(handle, value);
+        self.name_map.insert(path, handle);
+
+        Ok(handle)
+    }
+
+    /// Helper method for adding a value to the namespace at a path that is relative to the given
+    /// scope. This operation involves a lot of error handling in parts of the parser, so is
+    /// encapsulated here.
+    pub fn add_at_resolved_path(
+        &mut self,
+        path: AmlName,
+        scope: &AmlName,
+        value: AmlValue,
+    ) -> Result<AmlHandle, AmlError> {
+        self.add(path.resolve(scope)?, value)
+    }
+
+    pub fn get(&self, handle: AmlHandle) -> Result<&AmlValue, AmlError> {
+        self.object_map.get(&handle).ok_or(AmlError::HandleDoesNotExist(handle))
+    }
+
+    pub fn get_by_path(&self, path: &AmlName) -> Result<&AmlValue, AmlError> {
+        let handle = *self.name_map.get(path).ok_or(AmlError::ObjectDoesNotExist(path.as_string()))?;
+        self.get(handle).map_err(|_| AmlError::ObjectDoesNotExist(path.as_string()))
+    }
+
+    pub fn get_mut(&mut self, handle: AmlHandle) -> Result<&mut AmlValue, AmlError> {
+        self.object_map.get_mut(&handle).ok_or(AmlError::HandleDoesNotExist(handle))
+    }
+
+    pub fn get_by_path_mut(&mut self, path: &AmlName) -> Result<&mut AmlValue, AmlError> {
+        let handle = *self.name_map.get(path).ok_or(AmlError::ObjectDoesNotExist(path.as_string()))?;
+        self.get_mut(handle).map_err(|_| AmlError::ObjectDoesNotExist(path.as_string()))
+    }
+
+    /// Search for an object at the given path of the namespace, applying the search rules
+    /// described in §5.3 of the ACPI specification, if they are applicable. Returns the handle of
+    /// the first valid object, if found.
+    pub fn search(&self, path: &AmlName, starting_scope: &AmlName) -> Result<AmlHandle, AmlError> {
+        if path.search_rules_apply() {
+            /*
+             * If search rules apply, we need to recursively look through the namespace. If the
+             * given name does not occur in the current scope, we look at the parent scope, until
+             * we either find the name, or reach the root of the namespace.
+             */
+            let mut scope = starting_scope.clone();
+            assert!(scope.is_absolute());
+            loop {
+                // Search for the name at this namespace level. If we find it, we're done.
+                if let Some(handle) = self.name_map.get(&path.resolve(&scope)?) {
+                    return Ok(*handle);
+                }
+
+                // If we don't find it, go up a level in the namespace and search for it there,
+                // recursively.
+                match scope.parent() {
+                    Ok(parent) => scope = parent,
+                    // If we still haven't found the value and have run out of parents, return `None`.
+                    Err(AmlError::RootHasNoParent) => {
+                        return Err(AmlError::ObjectDoesNotExist(path.as_string()))
+                    }
+                    Err(err) => return Err(err),
+                }
+            }
+        } else {
+            // If search rules don't apply, simply resolve it against the starting scope
+            self.name_map
+                .get(&path.resolve(starting_scope)?)
+                .map(|&handle| handle)
+                .ok_or(AmlError::ObjectDoesNotExist(path.as_string()))
+        }
+    }
+}
+
+impl fmt::Debug for Namespace {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for (name, handle) in self.name_map.iter() {
+            write!(f, "{}: {:?}\n", name, self.object_map.get(handle).unwrap())?;
+        }
+
+        Ok(())
+    }
+}
+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub struct AmlName(pub(crate) Vec<NameComponent>);
+
+impl AmlName {
+    pub fn root() -> AmlName {
+        AmlName(alloc::vec![NameComponent::Root])
+    }
+
+    pub fn from_name_seg(seg: NameSeg) -> AmlName {
+        AmlName(alloc::vec![NameComponent::Segment(seg)])
+    }
+
+    /// Convert a string representation of an AML name into an `AmlName`. Returns `None` if the
+    /// passed string is not a valid AML path.
+    pub fn from_str(mut string: &str) -> Option<AmlName> {
+        if string.len() == 0 {
+            return None;
+        }
+
+        let mut components = Vec::new();
+
+        // If it starts with a \, make it an absolute name
+        if string.starts_with('\\') {
+            components.push(NameComponent::Root);
+            string = &string[1..];
+        }
+
+        if string.len() > 0 {
+            // Divide the rest of it into segments, and parse those
+            for mut part in string.split('.') {
+                // Handle prefix chars
+                while part.starts_with('^') {
+                    components.push(NameComponent::Prefix);
+                    part = &part[1..];
+                }
+
+                components.push(NameComponent::Segment(NameSeg::from_str(part)?));
+            }
+        }
+
+        Some(AmlName(components))
+    }
+
+    pub fn as_string(&self) -> String {
+        self.0
+            .iter()
+            .fold(String::new(), |name, component| match component {
+                NameComponent::Root => name + "\\",
+                NameComponent::Prefix => name + "^",
+                NameComponent::Segment(seg) => name + seg.as_str() + ".",
+            })
+            .trim_end_matches('.')
+            .to_string()
+    }
+
+    /// An AML path is normal if it does not contain any prefix elements ("^" characters, when
+    /// expressed as a string).
+    pub fn is_normal(&self) -> bool {
+        !self.0.contains(&NameComponent::Prefix)
+    }
+
+    pub fn is_absolute(&self) -> bool {
+        self.0.first() == Some(&NameComponent::Root)
+    }
+
+    /// Special rules apply when searching for certain paths (specifically, those that are made up
+    /// of a single name segment). Returns `true` if those rules apply.
+    pub fn search_rules_apply(&self) -> bool {
+        if self.0.len() != 1 {
+            return false;
+        }
+
+        match self.0[0] {
+            NameComponent::Segment(_) => true,
+            _ => false,
+        }
+    }
+
+    /// Normalize an AML path, resolving prefix chars. Returns `None` if the path normalizes to an
+    /// invalid path (e.g. `\^_FOO`)
+    pub fn normalize(self) -> Result<AmlName, AmlError> {
+        // TODO: currently, this doesn't do anything. Work out a nice way of handling prefix chars.
+        // If the name can't be normalized, emit AmlError::InvalidNormalizedName
+        Ok(self)
+    }
+
+    /// Get the parent of this `AmlName`. For example, the parent of `\_SB.PCI0._PRT` is `\_SB.PCI0`. The root
+    /// path has no parent, and so returns `None`.
+    pub fn parent(&self) -> Result<AmlName, AmlError> {
+        // Firstly, normalize the path so we don't have to deal with prefix chars
+        let mut normalized_self = self.clone().normalize()?;
+
+        match normalized_self.0.last() {
+            None | Some(NameComponent::Root) => Err(AmlError::RootHasNoParent),
+            Some(NameComponent::Segment(_)) => {
+                normalized_self.0.pop();
+                Ok(normalized_self)
+            }
+            Some(NameComponent::Prefix) => unreachable!(), // Prefix chars are removed by normalization
+        }
+    }
+
+    /// Resolve this path against a given scope, making it absolute. If the path is absolute, it is
+    /// returned directly. The path is also normalized.
+    pub fn resolve(&self, scope: &AmlName) -> Result<AmlName, AmlError> {
+        assert!(scope.is_absolute());
+
+        if self.is_absolute() {
+            return Ok(self.clone());
+        }
+
+        let mut resolved_path = scope.clone();
+        resolved_path.0.extend_from_slice(&(self.0));
+        resolved_path.normalize()
+    }
+}
+
+impl fmt::Display for AmlName {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.as_string())
+    }
+}
+
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Debug)]
+pub enum NameComponent {
+    Root,
+    Prefix,
+    Segment(NameSeg),
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_aml_name_from_str() {
+        assert_eq!(AmlName::from_str(""), None);
+        assert_eq!(AmlName::from_str("\\"), Some(AmlName::root()));
+        assert_eq!(
+            AmlName::from_str("\\_SB.PCI0"),
+            Some(AmlName(alloc::vec![
+                NameComponent::Root,
+                NameComponent::Segment(NameSeg([b'_', b'S', b'B', b'_'])),
+                NameComponent::Segment(NameSeg([b'P', b'C', b'I', b'0']))
+            ]))
+        );
+        assert_eq!(
+            AmlName::from_str("\\_SB.^^^PCI0"),
+            Some(AmlName(alloc::vec![
+                NameComponent::Root,
+                NameComponent::Segment(NameSeg([b'_', b'S', b'B', b'_'])),
+                NameComponent::Prefix,
+                NameComponent::Prefix,
+                NameComponent::Prefix,
+                NameComponent::Segment(NameSeg([b'P', b'C', b'I', b'0']))
+            ]))
+        );
+    }
+
+    #[test]
+    fn test_is_normal() {
+        assert_eq!(AmlName::root().is_normal(), true);
+        assert_eq!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_normal(), true);
+        assert_eq!(AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_normal(), false);
+        assert_eq!(AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_normal(), false);
+        assert_eq!(AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_normal(), false);
+        assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().is_normal(), true);
+    }
+
+    #[test]
+    fn test_is_absolute() {
+        assert_eq!(AmlName::root().is_absolute(), true);
+        assert_eq!(AmlName::from_str("\\_SB.PCI0.VGA").unwrap().is_absolute(), true);
+        assert_eq!(AmlName::from_str("\\_SB.^PCI0.VGA").unwrap().is_absolute(), true);
+        assert_eq!(AmlName::from_str("\\^_SB.^^PCI0.VGA").unwrap().is_absolute(), true);
+        assert_eq!(AmlName::from_str("_SB.^^PCI0.VGA").unwrap().is_absolute(), false);
+        assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().is_absolute(), false);
+    }
+
+    #[test]
+    fn test_search_rules_apply() {
+        assert_eq!(AmlName::root().search_rules_apply(), false);
+        assert_eq!(AmlName::from_str("\\_SB").unwrap().search_rules_apply(), false);
+        assert_eq!(AmlName::from_str("^VGA").unwrap().search_rules_apply(), false);
+        assert_eq!(AmlName::from_str("_SB.PCI0.VGA").unwrap().search_rules_apply(), false);
+        assert_eq!(AmlName::from_str("VGA").unwrap().search_rules_apply(), true);
+        assert_eq!(AmlName::from_str("_SB").unwrap().search_rules_apply(), true);
+    }
+
+    #[test]
+    fn test_aml_name_parent() {
+        assert_eq!(AmlName::from_str("\\").unwrap().parent(), Err(AmlError::RootHasNoParent));
+        assert_eq!(AmlName::from_str("\\_SB").unwrap().parent(), Ok(AmlName::root()));
+        assert_eq!(
+            AmlName::from_str("\\_SB.PCI0").unwrap().parent(),
+            Ok(AmlName::from_str("\\_SB").unwrap())
+        );
+        assert_eq!(AmlName::from_str("\\_SB.PCI0").unwrap().parent().unwrap().parent(), Ok(AmlName::root()));
+    }
+}

aml_parser/src/opcode.rs

@@ -32,6 +32,39 @@ pub const EXT_DEF_FIELD_OP: u8 = 0x81;
 pub const EXT_DEF_DEVICE_OP: u8 = 0x82;
 pub const EXT_DEF_PROCESSOR_OP: u8 = 0x83;
 
+/*
+ * Type 1 opcodes
+ */
+pub const DEF_IF_ELSE_OP: u8 = 0xa0;
+pub const DEF_ELSE_OP: u8 = 0xa1;
+pub const DEF_RETURN_OP: u8 = 0xa4;
+
+/*
+ * Type 2 opcodes
+ */
+pub const DEF_L_EQUAL_OP: u8 = 0x93;
+pub const DEF_STORE_OP: u8 = 0x70;
+
+/*
+ * Miscellaneous objects
+ */
+pub const EXT_DEBUG_OP: u8 = 0x31;
+pub const LOCAL0_OP: u8 = 0x60;
+pub const LOCAL1_OP: u8 = 0x61;
+pub const LOCAL2_OP: u8 = 0x62;
+pub const LOCAL3_OP: u8 = 0x63;
+pub const LOCAL4_OP: u8 = 0x64;
+pub const LOCAL5_OP: u8 = 0x65;
+pub const LOCAL6_OP: u8 = 0x66;
+pub const LOCAL7_OP: u8 = 0x67;
+pub const ARG0_OP: u8 = 0x68;
+pub const ARG1_OP: u8 = 0x69;
+pub const ARG2_OP: u8 = 0x6a;
+pub const ARG3_OP: u8 = 0x6b;
+pub const ARG4_OP: u8 = 0x6c;
+pub const ARG5_OP: u8 = 0x6d;
+pub const ARG6_OP: u8 = 0x6e;
+
 pub const EXT_OPCODE_PREFIX: u8 = 0x5b;
 
 pub(crate) fn opcode<'a, 'c>(opcode: u8) -> impl Parser<'a, 'c, ()>
@@ -41,7 +74,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)),
     }
 }
 
@@ -60,11 +93,7 @@ mod tests {
     #[test]
     fn empty() {
         let mut context = AmlContext::new();
-        check_err!(
-            opcode(NULL_NAME).parse(&[], &mut context),
-            AmlError::UnexpectedEndOfStream,
-            &[]
-        );
+        check_err!(opcode(NULL_NAME).parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         check_err!(
             ext_opcode(EXT_DEF_FIELD_OP).parse(&[], &mut context),
             AmlError::UnexpectedEndOfStream,
@@ -88,12 +117,11 @@ 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!(
-            ext_opcode(EXT_DEF_FIELD_OP)
-                .parse(&[EXT_OPCODE_PREFIX, EXT_DEF_FIELD_OP], &mut context),
+            ext_opcode(EXT_DEF_FIELD_OP).parse(&[EXT_OPCODE_PREFIX, EXT_DEF_FIELD_OP], &mut context),
             (),
             &[]
         );

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>,
@@ -70,6 +71,15 @@ where
     }
 }
 
+/// The identity parser - returns the stream and context unchanged. Useful for producing parsers
+/// that produce a result without parsing anything by doing: `id().map(|()| Ok(foo))`.
+pub fn id<'a, 'c>() -> impl Parser<'a, 'c, ()>
+where
+    'c: 'a,
+{
+    move |input: &'a [u8], context: &'c mut AmlContext| Ok((input, context, ()))
+}
+
 pub fn take<'a, 'c>() -> impl Parser<'a, 'c, u8>
 where
     'c: 'a,
@@ -205,13 +215,13 @@ where
 {
     move |input, context| {
         #[cfg(feature = "debug_parser")]
-        trace!("--> {}", scope_name);
+        log::trace!("--> {}", scope_name);
 
         // Return if the parse fails, so we don't print the tail. Makes it easier to debug.
         let (new_input, context, result) = parser.parse(input, context)?;
 
         #[cfg(feature = "debug_parser")]
-        trace!("<-- {}", scope_name);
+        log::trace!("<-- {}", scope_name);
 
         Ok((new_input, context, result))
     }
@@ -225,13 +235,13 @@ where
 {
     move |input, context| {
         #[cfg(feature = "debug_parser_verbose")]
-        trace!("--> {}", scope_name);
+        log::trace!("--> {}", scope_name);
 
         // Return if the parse fails, so we don't print the tail. Makes it easier to debug.
         let (new_input, context, result) = parser.parse(input, context)?;
 
         #[cfg(feature = "debug_parser_verbose")]
-        trace!("<-- {}", scope_name);
+        log::trace!("<-- {}", scope_name);
 
         Ok((new_input, context, result))
     }
@@ -255,12 +265,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)),
+        }
     }
 }
 
@@ -335,9 +344,7 @@ where
     P: Parser<'a, 'c, R>,
 {
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, ()> {
-        self.parser
-            .parse(input, context)
-            .map(|(new_input, new_context, _)| (new_input, new_context, ()))
+        self.parser.parse(input, context).map(|(new_input, new_context, _)| (new_input, new_context, ()))
     }
 }
 
@@ -360,9 +367,9 @@ where
 {
     fn parse(&self, input: &'a [u8], context: &'c mut AmlContext) -> ParseResult<'a, 'c, (R1, R2)> {
         self.p1.parse(input, context).and_then(|(next_input, context, result_a)| {
-            self.p2.parse(next_input, context).map(|(final_input, context, result_b)| {
-                (final_input, context, (result_a, result_b))
-            })
+            self.p2
+                .parse(next_input, context)
+                .map(|(final_input, context, result_b)| (final_input, context, (result_a, result_b)))
         })
     }
 }
@@ -395,19 +402,16 @@ where
     }
 }
 
-pub(crate) fn emit_no_parsers_could_parse<'a, 'c, R>() -> impl Parser<'a, 'c, R>
-where
-    'c: 'a,
-{
-    |input: &'a [u8], context| Err((input, context, AmlError::NoParsersCouldParse))
-}
-
 /// 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 macro choice {
+pub(crate) macro choice {
+    () => {
+        id().map(|()| Err(AmlError::WrongParser))
+    },
+
     ($first_parser: expr) => {
         $first_parser
-        .or(emit_no_parsers_could_parse())
+        .or(id().map(|()| Err(AmlError::WrongParser)))
     },
 
     ($first_parser: expr, $($other_parser: expr),*) => {
@@ -415,7 +419,7 @@ pub macro choice {
         $(
             .or($other_parser)
          )*
-        .or(emit_no_parsers_could_parse())
+        .or(id().map(|()| Err(AmlError::WrongParser)))
     }
 }
 
@@ -429,10 +433,22 @@ pub macro choice {
 ///     help: consider adding a a '#![recursion_limit="128"] attribute to your crate`
 /// Note: Increasing the recursion limit will not fix the issue, as the cycle will just continue
 /// until you either hit the new recursion limit or `rustc` overflows its stack.
-pub macro make_parser_concrete($parser: expr) {
+pub(crate) macro make_parser_concrete($parser: expr) {
     |input, context| ($parser).parse(input, context)
 }
 
+/// Helper macro for use within `map_with_context` as an alternative to "trying" an expression.
+///
+/// ### Example
+/// Problem: `expr?` won't work because the expected return type is `(Result<R, AmlError>, &mut AmlContext)`
+/// Solution: use `try_with_context!(context, expr)` instead.
+pub(crate) macro try_with_context($context: expr, $expr: expr) {
+    match $expr {
+        Ok(result) => result,
+        Err(err) => return (Err(err), $context),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -442,11 +458,7 @@ mod tests {
     fn test_take_n() {
         let mut context = AmlContext::new();
         check_err!(take_n(1).parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
-        check_err!(
-            take_n(2).parse(&[0xf5], &mut context),
-            AmlError::UnexpectedEndOfStream,
-            &[0xf5]
-        );
+        check_err!(take_n(2).parse(&[0xf5], &mut context), AmlError::UnexpectedEndOfStream, &[0xf5]);
 
         check_ok!(take_n(1).parse(&[0xff], &mut context), &[0xff], &[]);
         check_ok!(take_n(1).parse(&[0xff, 0xf8], &mut context), &[0xff], &[0xf8]);
@@ -456,11 +468,7 @@ mod tests {
     #[test]
     fn test_take_ux() {
         let mut context = AmlContext::new();
-        check_err!(
-            take_u16().parse(&[0x34], &mut context),
-            AmlError::UnexpectedEndOfStream,
-            &[0x34]
-        );
+        check_err!(take_u16().parse(&[0x34], &mut context), AmlError::UnexpectedEndOfStream, &[0x34]);
         check_ok!(take_u16().parse(&[0x34, 0x12], &mut context), 0x1234, &[]);
 
         check_err!(
@@ -468,20 +476,11 @@ mod tests {
             AmlError::UnexpectedEndOfStream,
             &[0x34, 0x12]
         );
-        check_ok!(
-            take_u32().parse(&[0x34, 0x12, 0xf4, 0xc3, 0x3e], &mut context),
-            0xc3f41234,
-            &[0x3e]
-        );
+        check_ok!(take_u32().parse(&[0x34, 0x12, 0xf4, 0xc3, 0x3e], &mut context), 0xc3f41234, &[0x3e]);
 
-        check_err!(
-            take_u64().parse(&[0x34], &mut context),
-            AmlError::UnexpectedEndOfStream,
-            &[0x34]
-        );
+        check_err!(take_u64().parse(&[0x34], &mut context), AmlError::UnexpectedEndOfStream, &[0x34]);
         check_ok!(
-            take_u64()
-                .parse(&[0x34, 0x12, 0x35, 0x76, 0xd4, 0x43, 0xa3, 0xb6, 0xff, 0x00], &mut context),
+            take_u64().parse(&[0x34, 0x12, 0x35, 0x76, 0xd4, 0x43, 0xa3, 0xb6, 0xff, 0x00], &mut context),
             0xb6a343d476351234,
             &[0xff, 0x00]
         );

aml_parser/src/pkg_length.rs

@@ -80,9 +80,7 @@ where
                  * The stream was too short. We return an error, making sure to return the
                  * *original* stream (that we haven't consumed any of).
                  */
-                Err((_, context, _)) => {
-                    return Err((input, context, AmlError::UnexpectedEndOfStream))
-                }
+                Err((_, context, _)) => return Err((input, context, AmlError::UnexpectedEndOfStream)),
             };
 
         Ok((new_input, context, length))
@@ -116,11 +114,7 @@ mod tests {
         let mut context = AmlContext::new();
         check_err!(pkg_length().parse(&[], &mut context), AmlError::UnexpectedEndOfStream, &[]);
         test_correct_pkglength(&[0x00], 0, &[]);
-        test_correct_pkglength(
-            &[0x05, 0xf5, 0x7f, 0x3e, 0x54, 0x03],
-            5,
-            &[0xf5, 0x7f, 0x3e, 0x54, 0x03],
-        );
+        test_correct_pkglength(&[0x05, 0xf5, 0x7f, 0x3e, 0x54, 0x03], 5, &[0xf5, 0x7f, 0x3e, 0x54, 0x03]);
         check_err!(
             pkg_length().parse(&[0b11000000, 0xff, 0x4f], &mut context),
             AmlError::UnexpectedEndOfStream,

aml_parser/src/term_object.rs

@@ -1,5 +1,7 @@
 use crate::{
-    name_object::{name_seg, name_string, AmlName},
+    misc::{arg_obj, local_obj},
+    name_object::{name_seg, name_string},
+    namespace::AmlName,
     opcode::{self, ext_opcode, opcode},
     parser::{
         choice,
@@ -11,10 +13,13 @@ use crate::{
         take_u16,
         take_u32,
         take_u64,
+        try_with_context,
         ParseResult,
         Parser,
     },
     pkg_length::{pkg_length, PkgLength},
+    type1::type1_opcode,
+    type2::type2_opcode,
     value::{AmlValue, FieldFlags, MethodFlags, RegionSpace},
     AmlContext,
     AmlError,
@@ -33,7 +38,9 @@ where
      */
     move |mut input: &'a [u8], mut context: &'c mut AmlContext| {
         while list_length.still_parsing(input) {
-            let (new_input, new_context, ()) = term_object().parse(input, context)?;
+            // TODO: currently, we ignore the value of the expression. We may need to propagate
+            // this.
+            let (new_input, new_context, _) = term_object().parse(input, context)?;
             input = new_input;
             context = new_context;
         }
@@ -42,14 +49,22 @@ where
     }
 }
 
-pub fn term_object<'a, 'c>() -> impl Parser<'a, 'c, ()>
+pub fn term_object<'a, 'c>() -> impl Parser<'a, 'c, Option<AmlValue>>
 where
     'c: 'a,
 {
     /*
      * TermObj := NamespaceModifierObj | NamedObj | Type1Opcode | Type2Opcode
      */
-    comment_scope_verbose("TermObj", choice!(namespace_modifier(), named_obj()))
+    comment_scope_verbose(
+        "TermObj",
+        choice!(
+            namespace_modifier().map(|()| Ok(None)),
+            named_obj().map(|()| Ok(None)),
+            type1_opcode().map(|()| Ok(None)),
+            type2_opcode().map(|value| Ok(Some(value)))
+        ),
+    )
 }
 
 pub fn namespace_modifier<'a, 'c>() -> impl Parser<'a, 'c, ()>
@@ -77,14 +92,7 @@ where
      */
     comment_scope_verbose(
         "NamedObj",
-        choice!(
-            def_op_region(),
-            def_field(),
-            def_method(),
-            def_device(),
-            def_processor(),
-            def_mutex()
-        ),
+        choice!(def_op_region(), def_field(), def_method(), def_device(), def_processor(), def_mutex()),
     )
 }
 
@@ -98,12 +106,17 @@ where
     opcode(opcode::DEF_NAME_OP)
         .then(comment_scope(
             "DefName",
-            name_string().then(data_ref_object()).map_with_context(
-                |(name, data_ref_object), context| {
-                    context.add_to_namespace(name, AmlValue::Name(box data_ref_object));
-                    (Ok(()), context)
-                },
-            ),
+            name_string().then(data_ref_object()).map_with_context(|(name, data_ref_object), context| {
+                try_with_context!(
+                    context,
+                    context.namespace.add_at_resolved_path(
+                        name,
+                        &context.current_scope,
+                        AmlValue::Name(box data_ref_object)
+                    )
+                );
+                (Ok(()), context)
+            }),
         ))
         .discard_result()
 }
@@ -122,13 +135,13 @@ where
                 .then(name_string())
                 .map_with_context(|(length, name), context| {
                     let previous_scope = context.current_scope.clone();
-                    context.current_scope = context.resolve_path(&name);
-                    (Ok((length, name, previous_scope)), context)
+                    context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
+                    (Ok((length, previous_scope)), context)
                 })
-                .feed(|(pkg_length, name, previous_scope)| {
-                    term_list(pkg_length).map(move |_| Ok((name.clone(), previous_scope.clone())))
+                .feed(|(pkg_length, previous_scope)| {
+                    term_list(pkg_length).map(move |_| Ok(previous_scope.clone()))
                 })
-                .map_with_context(|(name, previous_scope), context| {
+                .map_with_context(|previous_scope, context| {
                     context.current_scope = previous_scope;
                     (Ok(()), context)
                 }),
@@ -185,7 +198,14 @@ where
                         Err(err) => return (Err(err), context),
                     };
 
-                    context.add_to_namespace(name, AmlValue::OpRegion { region, offset, length });
+                    try_with_context!(
+                        context,
+                        context.namespace.add_at_resolved_path(
+                            name,
+                            &context.current_scope,
+                            AmlValue::OpRegion { region, offset, length }
+                        )
+                    );
                     (Ok(()), context)
                 },
             ),
@@ -204,32 +224,25 @@ where
     ext_opcode(opcode::EXT_DEF_FIELD_OP)
         .then(comment_scope(
             "DefField",
-            pkg_length().then(name_string()).then(take()).feed(
-                |((list_length, region_name), flags)| {
-                    move |mut input: &'a [u8],
-                          mut context: &'c mut AmlContext|
-                          -> ParseResult<'a, 'c, ()> {
-                        /*
-                         * FieldList := Nothing | <FieldElement FieldList>
-                         */
-                        // TODO: can this pattern be expressed as a combinator
-                        let mut current_offset = 0;
-                        while list_length.still_parsing(input) {
-                            let (new_input, new_context, field_length) = field_element(
-                                region_name.clone(),
-                                FieldFlags::new(flags),
-                                current_offset,
-                            )
-                            .parse(input, context)?;
-                            input = new_input;
-                            context = new_context;
-                            current_offset += field_length;
-                        }
-
-                        Ok((input, context, ()))
+            pkg_length().then(name_string()).then(take()).feed(|((list_length, region_name), flags)| {
+                move |mut input: &'a [u8], mut context: &'c mut AmlContext| -> ParseResult<'a, 'c, ()> {
+                    /*
+                     * FieldList := Nothing | <FieldElement FieldList>
+                     */
+                    // TODO: can this pattern be expressed as a combinator
+                    let mut current_offset = 0;
+                    while list_length.still_parsing(input) {
+                        let (new_input, new_context, field_length) =
+                            field_element(region_name.clone(), FieldFlags::new(flags), current_offset)
+                                .parse(input, context)?;
+                        input = new_input;
+                        context = new_context;
+                        current_offset += field_length;
                     }
-                },
-            ),
+
+                    Ok((input, context, ()))
+                }
+            }),
         ))
         .discard_result()
 }
@@ -266,9 +279,8 @@ where
      * Reserved fields shouldn't actually be added to the namespace; they seem to show gaps in
      * the operation region that aren't used for anything.
      */
-    let reserved_field = opcode(opcode::RESERVED_FIELD)
-        .then(pkg_length())
-        .map(|((), length)| Ok(length.raw_length as u64));
+    let reserved_field =
+        opcode(opcode::RESERVED_FIELD).then(pkg_length()).map(|((), length)| Ok(length.raw_length as u64));
 
     // TODO: work out what to do with an access field
     // let access_field = opcode(opcode::ACCESS_FIELD)
@@ -276,20 +288,23 @@ where
     //     .then(take())
     //     .map_with_context(|(((), access_type), access_attrib), context| (Ok(    , context));
 
-    let named_field =
-        name_seg().then(pkg_length()).map_with_context(move |(name_seg, length), context| {
-            context.add_to_namespace(
+    let named_field = name_seg().then(pkg_length()).map_with_context(move |(name_seg, length), context| {
+        try_with_context!(
+            context,
+            context.namespace.add_at_resolved_path(
                 AmlName::from_name_seg(name_seg),
+                &context.current_scope,
                 AmlValue::Field {
                     region: region_name.clone(),
                     flags,
                     offset: current_offset,
                     length: length.raw_length as u64,
                 },
-            );
+            )
+        );
 
-            (Ok(length.raw_length as u64), context)
-        });
+        (Ok(length.raw_length as u64), context)
+    });
 
     choice!(reserved_field, named_field)
 }
@@ -311,13 +326,16 @@ where
                 .then(name_string())
                 .then(take())
                 .feed(|((length, name), flags)| {
-                    take_to_end_of_pkglength(length)
-                        .map(move |code| Ok((name.clone(), flags, code)))
+                    take_to_end_of_pkglength(length).map(move |code| Ok((name.clone(), flags, code)))
                 })
                 .map_with_context(|(name, flags, code), context| {
-                    context.add_to_namespace(
-                        name,
-                        AmlValue::Method { flags: MethodFlags::new(flags), code: code.to_vec() },
+                    try_with_context!(
+                        context,
+                        context.namespace.add_at_resolved_path(
+                            name,
+                            &context.current_scope,
+                            AmlValue::Method { flags: MethodFlags::new(flags), code: code.to_vec() },
+                        )
                     );
                     (Ok(()), context)
                 }),
@@ -338,16 +356,21 @@ where
             pkg_length()
                 .then(name_string())
                 .map_with_context(|(length, name), context| {
-                    context.add_to_namespace(name.clone(), AmlValue::Device);
+                    try_with_context!(
+                        context,
+                        context.namespace.add_at_resolved_path(
+                            name.clone(),
+                            &context.current_scope,
+                            AmlValue::Device
+                        )
+                    );
 
                     let previous_scope = context.current_scope.clone();
-                    context.current_scope = context.resolve_path(&name);
+                    context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
 
                     (Ok((length, previous_scope)), context)
                 })
-                .feed(|(length, previous_scope)| {
-                    term_list(length).map(move |_| Ok(previous_scope.clone()))
-                })
+                .feed(|(length, previous_scope)| term_list(length).map(move |_| Ok(previous_scope.clone())))
                 .map_with_context(|previous_scope, context| {
                     context.current_scope = previous_scope;
                     (Ok(()), context)
@@ -374,13 +397,25 @@ where
                 .then(take())
                 .then(take_u32())
                 .then(take())
-                .feed(|((((pkg_length, name), proc_id), pblk_address), pblk_len)| {
-                    term_list(pkg_length)
-                        .map(move |_| Ok((name.clone(), proc_id, pblk_address, pblk_len)))
+                .map_with_context(|((((pkg_length, name), proc_id), pblk_address), pblk_len), context| {
+                    try_with_context!(
+                        context,
+                        context.namespace.add_at_resolved_path(
+                            name.clone(),
+                            &context.current_scope,
+                            AmlValue::Processor { id: proc_id, pblk_address, pblk_len }
+                        )
+                    );
+                    let previous_scope = context.current_scope.clone();
+                    context.current_scope = try_with_context!(context, name.resolve(&context.current_scope));
+
+                    (Ok((previous_scope, pkg_length)), context)
+                })
+                .feed(move |(previous_scope, pkg_length)| {
+                    term_list(pkg_length).map(move |_| Ok(previous_scope.clone()))
                 })
-                .map_with_context(|(name, id, pblk_address, pblk_len), context| {
-                    context
-                        .add_to_namespace(name, AmlValue::Processor { id, pblk_address, pblk_len });
+                .map_with_context(|previous_scope, context| {
+                    context.current_scope = previous_scope;
                     (Ok(()), context)
                 }),
         ))
@@ -400,7 +435,14 @@ where
         .then(comment_scope(
             "DefMutex",
             name_string().then(take()).map_with_context(|(name, sync_level), context| {
-                context.add_to_namespace(name, AmlValue::Mutex { sync_level });
+                try_with_context!(
+                    context,
+                    context.namespace.add_at_resolved_path(
+                        name,
+                        &context.current_scope,
+                        AmlValue::Mutex { sync_level }
+                    )
+                );
                 (Ok(()), context)
             }),
         ))
@@ -448,8 +490,7 @@ where
                     let mut package_contents = Vec::new();
 
                     while pkg_length.still_parsing(input) {
-                        let (new_input, new_context, value) =
-                            package_element().parse(input, context)?;
+                        let (new_input, new_context, value) = package_element().parse(input, context)?;
                         input = new_input;
                         context = new_context;
 
@@ -478,8 +519,19 @@ where
     /*
      * TermArg := Type2Opcode | DataObject | ArgObj | LocalObj
      */
-    // TODO: this doesn't yet parse Term2Opcode, ArgObj, or LocalObj
-    comment_scope_verbose("TermArg", choice!(data_object()))
+    comment_scope_verbose(
+        "TermArg",
+        choice!(
+            data_object(),
+            arg_obj().map_with_context(|arg_num, context| {
+                (Ok(try_with_context!(context, context.current_arg(arg_num)).clone()), context)
+            }),
+            local_obj().map_with_context(|local_num, context| {
+                (Ok(try_with_context!(context, context.local(local_num)).clone()), context)
+            }),
+            make_parser_concrete!(type2_opcode())
+        ),
+    )
 }
 
 pub fn data_ref_object<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
@@ -545,12 +597,12 @@ where
             opcode::BYTE_CONST => {
                 take().map(|value| Ok(AmlValue::Integer(value as u64))).parse(new_input, context)
             }
-            opcode::WORD_CONST => take_u16()
-                .map(|value| Ok(AmlValue::Integer(value as u64)))
-                .parse(new_input, context),
-            opcode::DWORD_CONST => take_u32()
-                .map(|value| Ok(AmlValue::Integer(value as u64)))
-                .parse(new_input, context),
+            opcode::WORD_CONST => {
+                take_u16().map(|value| Ok(AmlValue::Integer(value as u64))).parse(new_input, context)
+            }
+            opcode::DWORD_CONST => {
+                take_u32().map(|value| Ok(AmlValue::Integer(value as u64))).parse(new_input, context)
+            }
             opcode::QWORD_CONST => {
                 take_u64().map(|value| Ok(AmlValue::Integer(value))).parse(new_input, context)
             }
@@ -559,7 +611,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)),
         }
     };
 
@@ -624,8 +676,7 @@ mod test {
             &[0x28]
         );
         check_ok!(
-            computational_data()
-                .parse(&[0x0d, b'A', b'B', b'C', b'D', b'\0', 0xff, 0xf5], &mut context),
+            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_parser/src/test_utils.rs

@@ -2,9 +2,7 @@ pub(crate) macro check_err($parse: expr, $error: pat, $remains: expr) {
     match $parse {
         Ok(result) => panic!("Expected Err, got {:#?}", result),
         Err((remains, _, $error)) if *remains == *$remains => (),
-        Err((remains, _, $error)) => {
-            panic!("Correct error, incorrect stream returned: {:x?}", remains)
-        }
+        Err((remains, _, $error)) => panic!("Correct error, incorrect stream returned: {:x?}", remains),
         Err((_, _, err)) => panic!("Got wrong error: {:?}", err),
     }
 }

aml_parser/src/type1.rs

@@ -0,0 +1,92 @@
+use crate::{
+    opcode::{self, opcode},
+    parser::{choice, comment_scope, comment_scope_verbose, take_to_end_of_pkglength, ParseResult, Parser},
+    pkg_length::{pkg_length, PkgLength},
+    term_object::{term_arg, term_list},
+    AmlError,
+};
+
+/// Type 1 opcodes do not return a value and so can't be used in expressions.
+pub fn type1_opcode<'a, 'c>() -> impl Parser<'a, 'c, ()>
+where
+    'c: 'a,
+{
+    /*
+     * Type1Opcode := DefBreak | DefBreakPoint | DefContinue | DefFatal | DefIfElse | DefLoad | DefNoop |
+     *                DefNotify | DefRelease | DefReset | DefReturn | DefSignal | DefSleep | DefStall |
+     *                DefWhile
+     */
+    comment_scope_verbose("Type1Opcode", choice!(def_if_else(), def_return()))
+}
+
+fn def_if_else<'a, 'c>() -> impl Parser<'a, 'c, ()>
+where
+    'c: 'a,
+{
+    /*
+     * DefIfElse := 0xa0 PkgLength Predicate TermList DefElse
+     * Predicate := TermArg => Integer (0 = false, >0 = true)
+     * DefElse := Nothing | <0xa1 PkgLength TermList>
+     */
+    opcode(opcode::DEF_IF_ELSE_OP)
+        .then(comment_scope(
+            "DefIfElse",
+            pkg_length()
+                .then(term_arg())
+                .feed(|(length, predicate_arg)| {
+                    take_to_end_of_pkglength(length)
+                        .map(move |then_branch| Ok((predicate_arg.as_bool()?, then_branch)))
+                })
+                .then(choice!(
+                    opcode(opcode::DEF_ELSE_OP)
+                        .then(comment_scope_verbose(
+                            "DefElse",
+                            pkg_length().feed(|length| take_to_end_of_pkglength(length))
+                        ))
+                        .map(|((), else_branch): ((), &[u8])| Ok(else_branch)),
+                    |input, context| -> ParseResult<'a, 'c, &[u8]> {
+                        /*
+                         * This path parses an DefIfElse that doesn't have an else branch. We simply
+                         * return an empty slice, so if the predicate is false, we don't execute
+                         * anything.
+                         */
+                        Ok((input, context, &[]))
+                    }
+                ))
+                .map_with_context(|((predicate, then_branch), else_branch), context| {
+                    let branch = if predicate { then_branch } else { else_branch };
+
+                    match term_list(PkgLength::from_raw_length(branch, branch.len() as u32))
+                        .parse(branch, context)
+                    {
+                        Ok((_, context, result)) => (Ok(result), context),
+                        Err((_, context, err)) => (Err(err), context),
+                    }
+                }),
+        ))
+        .discard_result()
+}
+
+fn def_return<'a, 'c>() -> impl Parser<'a, 'c, ()>
+where
+    'c: 'a,
+{
+    /*
+     * DefReturn := 0xa4 ArgObject
+     * ArgObject := TermArg => DataRefObject
+     */
+    opcode(opcode::DEF_RETURN_OP)
+        .then(comment_scope_verbose(
+            "DefReturn",
+            term_arg().map(|return_arg| -> Result<(), AmlError> {
+                /*
+                 * To return a value, we want to halt execution of the method and propagate the
+                 * return value all the way up to the start of the method invocation. To do this,
+                 * we emit a special error that is intercepted during method invocation and turned
+                 * into a valid result.
+                 */
+                Err(AmlError::Return(return_arg))
+            }),
+        ))
+        .discard_result()
+}

aml_parser/src/type2.rs

@@ -0,0 +1,144 @@
+use crate::{
+    name_object::{name_string, super_name, Target},
+    opcode::{self, opcode},
+    parser::{choice, comment_scope, comment_scope_verbose, id, try_with_context, Parser},
+    term_object::term_arg,
+    value::AmlValue,
+    AmlError,
+};
+use alloc::boxed::Box;
+
+/// Type 2 opcodes return a value and so can be used in expressions.
+pub fn type2_opcode<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
+where
+    'c: 'a,
+{
+    /*
+     * Type2Opcode := DefAquire | DefAdd | DefAnd | DefBuffer | DefConcat | DefConcatRes |
+     *                DefCondRefOf | DefCopyObject | DefDecrement | DefDerefOf | DefDivide |
+     *                DefFindSetLeftBit | DefFindSetRightBit | DefFromBCD | DefIncrement | DefIndex |
+     *                DefLAnd | DefLEqual | DefLGreater | DefLGreaterEqual | DefLLess | DefLLessEqual |
+     *                DefMid | DefLNot | DefLNotEqual | DefLoadTable | DefLOr | DefMatch | DefMod |
+     *                DefMultiply | DefNAnd | DefNOr | DefNot | DefObjectType | DefOr | DefPackage |
+     *                DefVarPackage | DefRefOf | DefShiftLeft | DefShitRight | DefSizeOf | DefStore |
+     *                DefSubtract | DefTimer | DefToBCD | DefToBuffer | DefToDecimalString |
+     *                DefToHexString | DefToInteger | DefToString | DefWait | DefXOr | MethodInvocation
+     */
+    comment_scope_verbose("Type2Opcode", choice!(def_l_equal(), def_store(), method_invocation()))
+}
+
+fn def_l_equal<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
+where
+    'c: 'a,
+{
+    /*
+     * DefLEqual := 0x93 Operand Operand
+     * Operand := TermArg => Integer
+     */
+    opcode(opcode::DEF_L_EQUAL_OP)
+        .then(comment_scope_verbose(
+            "DefLEqual",
+            term_arg().then(term_arg()).map(|(left_arg, right_arg)| {
+                Ok(AmlValue::Boolean(left_arg.as_integer()? == right_arg.as_integer()?))
+            }),
+        ))
+        .map(|((), result)| Ok(result))
+}
+
+fn def_store<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
+where
+    'c: 'a,
+{
+    /*
+     * DefStore := 0x70 TermArg SuperName
+     *
+     * Implicit conversion is only applied when the destination target is a `Name` - not when we
+     * are storing into a method local or argument (these stores are semantically identical to
+     * CopyObject). We must also make sure to return a copy of the data that is in the destination
+     * after the store (as opposed to the data we think we put into it), because some stores can
+     * alter the data during the store.
+     */
+    opcode(opcode::DEF_STORE_OP)
+        .then(comment_scope("DefStore", term_arg().then(super_name())))
+        .map_with_context(|((), (value, target)), context| {
+            match target {
+                Target::Name(ref path) => {
+                    let handle =
+                        try_with_context!(context, context.namespace.search(path, &context.current_scope));
+                    let desired_type = context.namespace.get(handle).unwrap().type_of();
+                    let converted_object = try_with_context!(context, value.as_type(desired_type));
+
+                    *try_with_context!(context, context.namespace.get_mut(handle)) =
+                        AmlValue::Name(box converted_object);
+                    (Ok(context.namespace.get(handle).unwrap().clone()), context)
+                }
+
+                Target::Debug => {
+                    // TODO
+                    unimplemented!()
+                }
+
+                Target::Arg(arg_num) => {
+                    // TODO
+                    unimplemented!()
+                }
+
+                Target::Local(local_num) => {
+                    // TODO
+                    unimplemented!()
+                }
+            }
+        })
+}
+
+fn method_invocation<'a, 'c>() -> impl Parser<'a, 'c, AmlValue>
+where
+    'c: 'a,
+{
+    /*
+     * MethodInvocation := NameString TermArgList
+     *
+     * MethodInvocation is the worst of the AML structures, because you're meant to figure out how
+     * much you're meant to parse using the name of the method (by knowing from its definition how
+     * how many arguments it takes). However, the definition of a method can in theory appear after
+     * an invocation of that method, and so parsing them properly can be very difficult.
+     * NOTE: We don't support the case of the definition appearing after the invocation.
+     *
+     * It's also not clear whether things that aren't methods can be "invoked" using
+     * MethodInvocation with 0 arguments. It seems that references to DefNames can be encoded using
+     * MethodInvocation, at least, and should just be looked up.
+     */
+    comment_scope_verbose(
+        "MethodInvocation",
+        name_string()
+            .map_with_context(move |name, context| {
+                let handle =
+                    try_with_context!(context, context.namespace.search(&name, &context.current_scope))
+                        .clone();
+                (Ok(handle), context)
+            })
+            .feed(|handle| {
+                id().map_with_context(move |(), context| {
+                    let object = try_with_context!(context, context.namespace.get(handle));
+                    match object.clone() {
+                        AmlValue::Name(boxed_value) => (Ok(unbox(boxed_value)), context),
+
+                        AmlValue::Method { ref code, .. } => {
+                            // TODO: before we do this, we need to restructure the structures to allow us
+                            // to execute control methods from inside other control methods
+                            // TODO
+                            unimplemented!()
+                        }
+
+                        _ => (Err(AmlError::IncompatibleValueConversion), context),
+                    }
+                })
+            }),
+    )
+}
+
+/// An unfortunate helper method to unbox an owned, boxed value. `*x` is special-cased for `Box`
+/// here, but the compiler needs the type signature of the method to figure it out.
+fn unbox<T>(x: Box<T>) -> T {
+    *x
+}

aml_parser/src/value.rs

@@ -1,4 +1,4 @@
-use crate::{name_object::AmlName, AmlError};
+use crate::{misc::ArgNum, namespace::AmlName, AmlError};
 use alloc::{boxed::Box, string::String, vec::Vec};
 use bit_field::BitField;
 
@@ -91,8 +91,33 @@ impl MethodFlags {
     }
 }
 
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
+pub enum AmlType {
+    Uninitialized,
+    Buffer,
+    BufferField,
+    /// Handle to a definition block handle. Returned by the `Load` operator.
+    DdbHandle,
+    DebugObject,
+    Device,
+    Event,
+    FieldUnit,
+    Integer,
+    Method,
+    Mutex,
+    ObjReference,
+    OpRegion,
+    Package,
+    PowerResource,
+    Processor,
+    RawDataBuffer,
+    String,
+    ThermalZone,
+}
+
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum AmlValue {
+    Boolean(bool),
     Integer(u64),
     String(String),
     Name(Box<AmlValue>),
@@ -107,11 +132,36 @@ pub enum AmlValue {
 }
 
 impl AmlValue {
+    /// Returns the AML type of this value. For `Name`, this returns the type of the inner value.
+    pub fn type_of(&self) -> AmlType {
+        match self {
+            AmlValue::Boolean(_) => AmlType::Integer,
+            AmlValue::Integer(_) => AmlType::Integer,
+            AmlValue::String(_) => AmlType::String,
+            AmlValue::Name(boxed_value) => boxed_value.type_of(),
+            AmlValue::OpRegion { .. } => AmlType::OpRegion,
+            AmlValue::Field { .. } => AmlType::FieldUnit,
+            AmlValue::Device => AmlType::Device,
+            AmlValue::Method { .. } => AmlType::Method,
+            AmlValue::Buffer { .. } => AmlType::Buffer,
+            AmlValue::Processor { .. } => AmlType::Processor,
+            AmlValue::Mutex { .. } => AmlType::Mutex,
+            AmlValue::Package(_) => AmlType::Package,
+        }
+    }
+
+    pub fn as_bool(&self) -> Result<bool, AmlError> {
+        match self {
+            AmlValue::Boolean(value) => Ok(*value),
+            _ => Err(AmlError::IncompatibleValueConversion),
+        }
+    }
+
     pub fn as_integer(&self) -> Result<u64, AmlError> {
         match self {
             AmlValue::Integer(value) => Ok(*value),
 
-            AmlValue::Buffer { size, ref bytes } => {
+            AmlValue::Buffer { ref bytes, .. } => {
                 /*
                  * "The first 8 bytes of the buffer are converted to an integer, taking the first
                  * byte as the least significant byte of the integer. A zero-length buffer is
@@ -132,4 +182,62 @@ impl AmlValue {
             _ => Err(AmlError::IncompatibleValueConversion),
         }
     }
+
+    /// Convert this value to a value of the same data, but with the given AML type, if possible,
+    /// by converting the implicit conversions described in §19.3.5 of the spec.
+    ///
+    /// The implicit conversions applied are:
+    ///     `Buffer` from: `Integer`, `String`, `Debug`
+    ///     `BufferField` from: `Integer`, `Buffer`, `String`, `Debug`
+    ///     `DdbHandle` from: `Integer`, `Debug`
+    ///     `FieldUnit` from: `Integer`,`Buffer`, `String`, `Debug`
+    ///     `Integer` from: `Buffer`, `BufferField`, `DdbHandle`, `FieldUnit`, `String`, `Debug`
+    ///     `Package` from: `Debug`
+    ///     `String` from: `Integer`, `Buffer`, `Debug`
+    pub fn as_type(&self, desired_type: AmlType) -> Result<AmlValue, AmlError> {
+        // Cache the type of this object
+        let our_type = self.type_of();
+
+        // If the value is already of the correct type, just return it as is
+        if our_type == desired_type {
+            return Ok(self.clone());
+        }
+
+        // TODO: implement all of the rules
+        match desired_type {
+            AmlType::Integer => self.as_integer().map(|value| AmlValue::Integer(value)),
+            _ => Err(AmlError::IncompatibleValueConversion),
+        }
+    }
+}
+
+/// A control method can take up to 7 arguments, each of which can be an `AmlValue`.
+#[derive(Clone, Debug, Default)]
+pub struct Args {
+    pub arg_0: Option<AmlValue>,
+    pub arg_1: Option<AmlValue>,
+    pub arg_2: Option<AmlValue>,
+    pub arg_3: Option<AmlValue>,
+    pub arg_4: Option<AmlValue>,
+    pub arg_5: Option<AmlValue>,
+    pub arg_6: Option<AmlValue>,
+}
+
+impl Args {
+    /// Get an argument by its `ArgNum`.
+    ///
+    /// ### Panics
+    /// Panics if passed an invalid argument number (valid argument numbers are `0..=6`)
+    pub fn arg(&self, num: ArgNum) -> Result<&AmlValue, AmlError> {
+        match num {
+            0 => self.arg_0.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            1 => self.arg_1.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            2 => self.arg_2.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            3 => self.arg_3.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            4 => self.arg_4.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            5 => self.arg_5.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            6 => self.arg_6.as_ref().ok_or(AmlError::InvalidArgumentAccess(num)),
+            _ => panic!("Invalid argument number: {}", num),
+        }
+    }
 }

rustfmt.toml

@@ -7,5 +7,6 @@ use_field_init_shorthand = true
 use_try_shorthand = true
 format_doc_comments = true
 wrap_comments = true
-comment_width = 100
+max_width = 110
+comment_width = 110
 use_small_heuristics = "max"