cargo clippy --fix

This adds some new directives for clippy and fixes all warnings.
Also updated the dependencies to their latest versions.

.gitignore

@@ -66,3 +66,4 @@ TAGS.vi
 src/.DS_Store
 tmp.*.rs
 .vscode
+Cargo.lock

Cargo.toml

@@ -40,8 +40,8 @@ edition = "2021"
 name = "mos6502"
 
 [dependencies]
-bitflags = "2.3.3"
-log = "0.4.19"
+bitflags = "2.5.0"
+log = "0.4.21"
 
 [features]
 decimal_mode = []

src/cpu.rs

@@ -47,6 +47,10 @@ where
 }
 
 impl<M: Bus, V: Variant> CPU<M, V> {
+    // Allowing `needless_pass_by_value` to simplify construction. Passing by
+    // value avoids the borrow and improves readability when constructing the
+    // CPU.
+    #[allow(clippy::needless_pass_by_value)]
     pub fn new(memory: M, _variant: V) -> CPU<M, V> {
         CPU {
             registers: Registers::new(),
@@ -56,10 +60,23 @@ impl<M: Bus, V: Variant> CPU<M, V> {
     }
 
     pub fn reset(&mut self) {
-        //TODO: // should read some bytes from the stack and also get the PC from the reset vector
+        //TODO: should read some bytes from the stack and also get the PC from the reset vector
     }
 
+    /// Get the next byte from memory and decode it into an instruction and addressing mode.
+    ///
+    /// # Panics
+    ///
+    /// This function will panic if the instruction is not recognized
+    /// (i.e. the opcode is invalid or has not been implemented).
     pub fn fetch_next_and_decode(&mut self) -> Option<DecodedInstr> {
+        // Helper function to read a 16-bit address from memory
+        fn read_address<M: Bus>(mem: &mut M, addr: u16) -> [u8; 2] {
+            let lo = mem.get_byte(addr);
+            let hi = mem.get_byte(addr.wrapping_add(1));
+            [lo, hi]
+        }
+
         let x: u8 = self.memory.get_byte(self.registers.program_counter);
 
         match V::decode(x) {
@@ -87,12 +104,6 @@ impl<M: Bus, V: Variant> CPU<M, V> {
 
                 let memory = &mut self.memory;
 
-                fn read_address<M: Bus>(mem: &mut M, addr: u16) -> [u8; 2] {
-                    let lo = mem.get_byte(addr);
-                    let hi = mem.get_byte(addr.wrapping_add(1));
-                    [lo, hi]
-                }
-
                 let am_out = match am {
                     AddressingMode::Accumulator | AddressingMode::Implied => {
                         // Always the same -- no input
@@ -201,6 +212,14 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                             address_from_bytes(slice[0], slice[1]).wrapping_add(y.into()),
                         )
                     }
+                    AddressingMode::ZeroPageIndirect => {
+                        // Use [u8, ..1] from instruction
+                        // This is where the absolute (16-bit) target address is stored.
+                        // (Output: a 16-bit address)
+                        let start = slice[0];
+                        let slice = read_address(memory, u16::from(start));
+                        OpInput::UseAddress(address_from_bytes(slice[0], slice[1]))
+                    }
                 };
 
                 // Increment program counter
@@ -213,24 +232,25 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     pub fn execute_instruction(&mut self, decoded_instr: DecodedInstr) {
         match decoded_instr {
             (Instruction::ADC, OpInput::UseImmediate(val)) => {
-                debug!("add with carry immediate: {}", val);
+                log::debug!("add with carry immediate: {}", val);
                 self.add_with_carry(val);
             }
             (Instruction::ADC, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("add with carry. address: {:?}. value: {}", addr, val);
+                log::debug!("add with carry. address: {:?}. value: {}", addr, val);
                 self.add_with_carry(val);
             }
             (Instruction::ADCnd, OpInput::UseImmediate(val)) => {
-                debug!("add with carry immediate: {}", val);
+                log::debug!("add with carry immediate: {}", val);
                 self.add_with_no_decimal(val);
             }
             (Instruction::ADCnd, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("add with carry. address: {:?}. value: {}", addr, val);
+                log::debug!("add with carry. address: {:?}. value: {}", addr, val);
                 self.add_with_no_decimal(val);
             }
 
@@ -274,6 +294,16 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 self.branch_if_not_equal(addr);
             }
 
+            (Instruction::BIT, OpInput::UseImmediate(val)) => {
+                self.registers.status.set_with_mask(
+                    Status::PS_ZERO,
+                    Status::new(StatusArgs {
+                        zero: 0 == (self.registers.accumulator & val),
+                        ..StatusArgs::none()
+                    }),
+                );
+            }
+
             (Instruction::BIT, OpInput::UseAddress(addr)) => {
                 let a: u8 = self.registers.accumulator;
                 let m: u8 = self.memory.get_byte(addr);
@@ -301,7 +331,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
 
             (Instruction::BMI, OpInput::UseRelative(rel)) => {
                 let addr = self.registers.program_counter.wrapping_add(rel);
-                debug!("branch if minus relative. address: {:?}", addr);
+                log::debug!("branch if minus relative. address: {:?}", addr);
                 self.branch_if_minus(addr);
             }
 
@@ -310,6 +340,11 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 self.branch_if_positive(addr);
             }
 
+            (Instruction::BRA, OpInput::UseRelative(rel)) => {
+                let addr = self.registers.program_counter.wrapping_add(rel);
+                self.branch(addr);
+            }
+
             (Instruction::BRK, OpInput::UseImplied) => {
                 for b in self.registers.program_counter.wrapping_sub(1).to_be_bytes() {
                     self.push_on_stack(b);
@@ -317,10 +352,22 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 self.push_on_stack(self.registers.status.bits());
                 let pcl = self.memory.get_byte(0xfffe);
                 let pch = self.memory.get_byte(0xffff);
-                self.jump(((pch as u16) << 8) | pcl as u16);
+                self.jump((u16::from(pch) << 8) | u16::from(pcl));
                 self.registers.status.or(Status::PS_DISABLE_INTERRUPTS);
             }
 
+            (Instruction::BRKcld, OpInput::UseImplied) => {
+                for b in self.registers.program_counter.wrapping_sub(1).to_be_bytes() {
+                    self.push_on_stack(b);
+                }
+                self.push_on_stack(self.registers.status.bits());
+                let pcl = self.memory.get_byte(0xfffe);
+                let pch = self.memory.get_byte(0xffff);
+                self.jump((u16::from(pch) << 8) | u16::from(pcl));
+                self.registers.status.or(Status::PS_DISABLE_INTERRUPTS);
+                self.registers.status.and(!Status::PS_DECIMAL_MODE);
+            }
+
             (Instruction::BVC, OpInput::UseRelative(rel)) => {
                 let addr = self.registers.program_counter.wrapping_add(rel);
                 self.branch_if_overflow_clear(addr);
@@ -412,32 +459,32 @@ impl<M: Bus, V: Variant> CPU<M, V> {
             }
 
             (Instruction::LDA, OpInput::UseImmediate(val)) => {
-                debug!("load A immediate: {}", val);
+                log::debug!("load A immediate: {}", val);
                 self.load_accumulator(val);
             }
             (Instruction::LDA, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("load A. address: {:?}. value: {}", addr, val);
+                log::debug!("load A. address: {:?}. value: {}", addr, val);
                 self.load_accumulator(val);
             }
 
             (Instruction::LDX, OpInput::UseImmediate(val)) => {
-                debug!("load X immediate: {}", val);
+                log::debug!("load X immediate: {}", val);
                 self.load_x_register(val);
             }
             (Instruction::LDX, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("load X. address: {:?}. value: {}", addr, val);
+                log::debug!("load X. address: {:?}. value: {}", addr, val);
                 self.load_x_register(val);
             }
 
             (Instruction::LDY, OpInput::UseImmediate(val)) => {
-                debug!("load Y immediate: {}", val);
+                log::debug!("load Y immediate: {}", val);
                 self.load_y_register(val);
             }
             (Instruction::LDY, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("load Y. address: {:?}. value: {}", addr, val);
+                log::debug!("load Y. address: {:?}. value: {}", addr, val);
                 self.load_y_register(val);
             }
 
@@ -466,11 +513,47 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 let val = self.registers.accumulator;
                 self.push_on_stack(val);
             }
+            (Instruction::PHX, OpInput::UseImplied) => {
+                // Push X
+                self.push_on_stack(self.registers.index_x);
+            }
+            (Instruction::PHY, OpInput::UseImplied) => {
+                // Push Y
+                self.push_on_stack(self.registers.index_y);
+            }
             (Instruction::PHP, OpInput::UseImplied) => {
                 // Push status
                 let val = self.registers.status.bits() | 0x30;
                 self.push_on_stack(val);
             }
+            (Instruction::PLX, OpInput::UseImplied) => {
+                // Pull accumulator
+                self.pull_from_stack();
+                let val: u8 = self.fetch_from_stack();
+                self.registers.index_x = val;
+                self.registers.status.set_with_mask(
+                    Status::PS_ZERO | Status::PS_NEGATIVE,
+                    Status::new(StatusArgs {
+                        zero: val == 0,
+                        negative: self.registers.accumulator > 127,
+                        ..StatusArgs::none()
+                    }),
+                );
+            }
+            (Instruction::PLY, OpInput::UseImplied) => {
+                // Pull accumulator
+                self.pull_from_stack();
+                let val: u8 = self.fetch_from_stack();
+                self.registers.index_y = val;
+                self.registers.status.set_with_mask(
+                    Status::PS_ZERO | Status::PS_NEGATIVE,
+                    Status::new(StatusArgs {
+                        zero: val == 0,
+                        negative: self.registers.accumulator > 127,
+                        ..StatusArgs::none()
+                    }),
+                );
+            }
             (Instruction::PLA, OpInput::UseImplied) => {
                 // Pull accumulator
                 self.pull_from_stack();
@@ -527,32 +610,33 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 self.registers.status = Status::from_bits_truncate(val);
                 let pcl: u8 = self.pull_from_stack();
                 let pch: u8 = self.fetch_from_stack();
-                self.registers.program_counter = ((pch as u16) << 8) | pcl as u16;
+                self.registers.program_counter = (u16::from(pch) << 8) | u16::from(pcl);
             }
             (Instruction::RTS, OpInput::UseImplied) => {
                 self.pull_from_stack();
                 let pcl: u8 = self.pull_from_stack();
                 let pch: u8 = self.fetch_from_stack();
-                self.registers.program_counter = (((pch as u16) << 8) | pcl as u16).wrapping_add(1);
+                self.registers.program_counter =
+                    ((u16::from(pch) << 8) | u16::from(pcl)).wrapping_add(1);
             }
 
             (Instruction::SBC, OpInput::UseImmediate(val)) => {
-                debug!("subtract with carry immediate: {}", val);
+                log::debug!("subtract with carry immediate: {}", val);
                 self.subtract_with_carry(val);
             }
             (Instruction::SBC, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("subtract with carry. address: {:?}. value: {}", addr, val);
+                log::debug!("subtract with carry. address: {:?}. value: {}", addr, val);
                 self.subtract_with_carry(val);
             }
 
             (Instruction::SBCnd, OpInput::UseImmediate(val)) => {
-                debug!("subtract with carry immediate: {}", val);
+                log::debug!("subtract with carry immediate: {}", val);
                 self.subtract_with_no_decimal(val);
             }
             (Instruction::SBCnd, OpInput::UseAddress(addr)) => {
                 let val = self.memory.get_byte(addr);
-                debug!("subtract with carry. address: {:?}. value: {}", addr, val);
+                log::debug!("subtract with carry. address: {:?}. value: {}", addr, val);
                 self.subtract_with_no_decimal(val);
             }
 
@@ -575,6 +659,9 @@ impl<M: Bus, V: Variant> CPU<M, V> {
             (Instruction::STY, OpInput::UseAddress(addr)) => {
                 self.memory.set_byte(addr, self.registers.index_y);
             }
+            (Instruction::STZ, OpInput::UseAddress(addr)) => {
+                self.memory.set_byte(addr, 0);
+            }
 
             (Instruction::TAX, OpInput::UseImplied) => {
                 let val = self.registers.accumulator;
@@ -584,6 +671,38 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 let val = self.registers.accumulator;
                 self.load_y_register(val);
             }
+            (Instruction::TRB, OpInput::UseAddress(addr)) => {
+                let val = self.memory.get_byte(addr);
+
+                // The zero flag is set based on the result of the 'and'.
+                self.registers.status.set_with_mask(
+                    Status::PS_ZERO,
+                    Status::new(StatusArgs {
+                        zero: 0 == (self.registers.accumulator & val),
+                        ..StatusArgs::none()
+                    }),
+                );
+
+                // The 1's in the accumulator set the corresponding bits in the operand
+                let res = self.registers.accumulator | val;
+                self.memory.set_byte(addr, res);
+            }
+            (Instruction::TSB, OpInput::UseAddress(addr)) => {
+                let val = self.memory.get_byte(addr);
+
+                // The zero flag is set based on the result of the 'and'.
+                self.registers.status.set_with_mask(
+                    Status::PS_ZERO,
+                    Status::new(StatusArgs {
+                        zero: 0 == (self.registers.accumulator & val),
+                        ..StatusArgs::none()
+                    }),
+                );
+
+                // The 1's in the accumulator clear the corresponding bits in the operand
+                let res = (self.registers.accumulator ^ 0xff) & val;
+                self.memory.set_byte(addr, res);
+            }
             (Instruction::TSX, OpInput::UseImplied) => {
                 let StackPointer(val) = self.registers.stack_pointer;
                 self.load_x_register(val);
@@ -605,10 +724,10 @@ impl<M: Bus, V: Variant> CPU<M, V> {
             }
 
             (Instruction::NOP, OpInput::UseImplied) => {
-                debug!("NOP instruction");
+                log::debug!("NOP instruction");
             }
             (_, _) => {
-                debug!(
+                log::debug!(
                     "attempting to execute unimplemented or invalid \
                      instruction"
                 );
@@ -628,23 +747,23 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         }
     }
 
-    fn set_flags_from_i8(status: &mut Status, value: i8) {
-        let is_zero = value == 0;
-        let is_negative = value < 0;
-
-        status.set_with_mask(
-            Status::PS_ZERO | Status::PS_NEGATIVE,
-            Status::new(StatusArgs {
-                zero: is_zero,
-                negative: is_negative,
-                ..StatusArgs::none()
-            }),
-        );
+    /// Checks if a given `u8` value should be interpreted as negative when
+    /// considered as `i8`.
+    ///
+    /// In an 8-bit unsigned integer (`u8`), values range from 0 to 255. When
+    /// these values are interpreted as signed integers (`i8`), values from 128
+    /// to 255 are considered negative, corresponding to the signed range -128
+    /// to -1. This function checks if the provided `u8` value falls within that
+    /// range, effectively determining if the most significant bit is set, which
+    /// indicates a negative number in two's complement form.
+    /// ```
+    const fn value_is_negative(value: u8) -> bool {
+        value > 127
     }
 
     fn set_flags_from_u8(status: &mut Status, value: u8) {
         let is_zero = value == 0;
-        let is_negative = value > 127;
+        let is_negative = Self::value_is_negative(value);
 
         status.set_with_mask(
             Status::PS_ZERO | Status::PS_NEGATIVE,
@@ -668,7 +787,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 ..StatusArgs::none()
             }),
         );
-        CPU::<M, V>::set_flags_from_i8(status, *p_val as i8);
+        CPU::<M, V>::set_flags_from_u8(status, *p_val);
     }
 
     fn shift_right_with_flags(p_val: &mut u8, status: &mut Status) {
@@ -682,7 +801,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 ..StatusArgs::none()
             }),
         );
-        CPU::<M, V>::set_flags_from_i8(status, *p_val as i8);
+        CPU::<M, V>::set_flags_from_u8(status, *p_val);
     }
 
     fn rotate_left_with_flags(p_val: &mut u8, status: &mut Status) {
@@ -698,7 +817,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 ..StatusArgs::none()
             }),
         );
-        CPU::<M, V>::set_flags_from_i8(status, *p_val as i8);
+        CPU::<M, V>::set_flags_from_u8(status, *p_val);
     }
 
     fn rotate_right_with_flags(p_val: &mut u8, status: &mut Status) {
@@ -714,7 +833,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                 ..StatusArgs::none()
             }),
         );
-        CPU::<M, V>::set_flags_from_i8(status, *p_val as i8);
+        CPU::<M, V>::set_flags_from_u8(status, *p_val);
     }
 
     fn set_u8_with_flags(mem: &mut u8, status: &mut Status, value: u8) {
@@ -747,7 +866,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
     }
 
     fn add_with_carry(&mut self, value: u8) {
-        fn decimal_adjust(result: u8) -> u8 {
+        const fn decimal_adjust(result: u8) -> u8 {
             let bcd1: u8 = if (result & 0x0f) > 0x09 { 0x06 } else { 0x00 };
 
             let bcd2: u8 = if (result.wrapping_add(bcd1) & 0xf0) > 0x90 {
@@ -791,7 +910,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
 
         self.load_accumulator(result);
 
-        debug!("accumulator: {}", self.registers.accumulator);
+        log::debug!("accumulator: {}", self.registers.accumulator);
     }
 
     fn add_with_no_decimal(&mut self, value: u8) {
@@ -823,7 +942,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
 
         self.load_accumulator(result);
 
-        debug!("accumulator: {}", self.registers.accumulator);
+        log::debug!("accumulator: {}", self.registers.accumulator);
     }
 
     fn and(&mut self, value: u8) {
@@ -835,11 +954,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         // A - M - (1 - C)
 
         // nc -- 'not carry'
-        let nc: u8 = if self.registers.status.contains(Status::PS_CARRY) {
-            0
-        } else {
-            1
-        };
+        let nc: u8 = u8::from(!self.registers.status.contains(Status::PS_CARRY));
 
         let a_before = self.registers.accumulator;
 
@@ -881,11 +996,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         // A - M - (1 - C)
 
         // nc -- 'not carry'
-        let nc: u8 = if self.registers.status.contains(Status::PS_CARRY) {
-            0
-        } else {
-            1
-        };
+        let nc: u8 = u8::from(!self.registers.status.contains(Status::PS_CARRY));
 
         let a_before = self.registers.accumulator;
 
@@ -943,13 +1054,12 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         let value_new = val.wrapping_add(1);
         *val = value_new;
 
-        let is_negative = (value_new as i8) < 0;
         let is_zero = value_new == 0;
 
         flags.set_with_mask(
             Status::PS_NEGATIVE | Status::PS_ZERO,
             Status::new(StatusArgs {
-                negative: is_negative,
+                negative: Self::value_is_negative(value_new),
                 zero: is_zero,
                 ..StatusArgs::none()
             }),
@@ -960,14 +1070,14 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         let value_new = val.wrapping_sub(1);
         *val = value_new;
 
-        let is_negative = (value_new as i8) < 0;
         let is_zero = value_new == 0;
+        let is_negative = Self::value_is_negative(value_new);
 
         flags.set_with_mask(
             Status::PS_NEGATIVE | Status::PS_ZERO,
             Status::new(StatusArgs {
-                negative: is_negative,
                 zero: is_zero,
+                negative: is_negative,
                 ..StatusArgs::none()
             }),
         );
@@ -1007,6 +1117,10 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         }
     }
 
+    fn branch(&mut self, addr: u16) {
+        self.registers.program_counter = addr;
+    }
+
     fn branch_if_positive(&mut self, addr: u16) {
         if !self.registers.status.contains(Status::PS_NEGATIVE) {
             self.registers.program_counter = addr;
@@ -1033,20 +1147,24 @@ impl<M: Bus, V: Variant> CPU<M, V> {
     //   ...
     //   The N flag contains most significant bit of the subtraction result.
     fn compare(&mut self, r: u8, val: u8) {
+        // Setting the CARRY flag: A (unsigned) >= NUM (unsigned)
         if r >= val {
             self.registers.status.insert(Status::PS_CARRY);
         } else {
             self.registers.status.remove(Status::PS_CARRY);
         }
 
+        // Setting the ZERO flag: A = NUM
         if r == val {
             self.registers.status.insert(Status::PS_ZERO);
         } else {
             self.registers.status.remove(Status::PS_ZERO);
         }
 
-        let diff: i8 = (r as i8).wrapping_sub(val as i8);
-        if diff < 0 {
+        // Set the NEGATIVE flag based on the MSB of the result of subtraction
+        // This checks if the 8th bit is set (0x80 in hex is 128 in decimal, which is the 8th bit in a byte)
+        let diff = r.wrapping_sub(val);
+        if Self::value_is_negative(diff) {
             self.registers.status.insert(Status::PS_NEGATIVE);
         } else {
             self.registers.status.remove(Status::PS_NEGATIVE);
@@ -1059,7 +1177,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
     }
 
     fn compare_with_x_register(&mut self, val: u8) {
-        debug!("compare_with_x_register");
+        log::debug!("compare_with_x_register");
 
         let x = self.registers.index_x;
         self.compare(x, val);
@@ -1112,6 +1230,11 @@ impl<M: Bus, V: Variant> core::fmt::Debug for CPU<M, V> {
 
 #[cfg(test)]
 mod tests {
+    // Casting from signed to unsigned integers is intentional in these tests
+    #![allow(clippy::cast_sign_loss)]
+    // Operations may intentionally wrap due to emulation of 8-bit unsigned
+    // integer arithmetic. We do this to test wrap-around conditions.
+    #![allow(clippy::cast_possible_wrap)]
 
     use super::*;
     use crate::instruction::Nmos6502;
@@ -1215,6 +1338,8 @@ mod tests {
         assert!(!cpu.registers.status.contains(Status::PS_NEGATIVE));
         assert!(!cpu.registers.status.contains(Status::PS_OVERFLOW));
 
+        // Allow casting from i8 to u8; -127i8 wraps to 129u8, as intended for
+        // two's complement arithmetic.
         cpu.add_with_carry(-127i8 as u8);
         assert_eq!(cpu.registers.accumulator, 0);
         assert!(cpu.registers.status.contains(Status::PS_CARRY));

src/instruction.rs

@@ -25,89 +25,211 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
-// Abbreviations
-//
-// General
-//
-//        M | `Memory location`
-//
-// Registers
-//
-//        A | accumulator
-//        X | general purpose register
-//        Y | general purpose register
-//        F | processor status flags, collectively
-// NV-BDIZC | processor status flags, individually
-//        S | stack pointer
-//       PC | program counter
-//
-
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub enum Instruction {
-    ADC,   // ADd with Carry................ | NV ...ZC A            = A + M + C
-    ADCnd, // ADd with Carry................ | NV ...ZC A            = A + M + C
-    AND,   // logical AND (bitwise)......... | N. ...Z. A            = A && M
-    ASL,   // Arithmetic Shift Left......... | N. ...ZC A            = M << 1
-    BCC,   // Branch if Carry Clear......... | .. .....         PC   = !C
-    BCS,   // Branch if Carry Set........... | .. .....         PC   = C
-    BEQ,   // Branch if Equal (to zero?).... | .. .....         PC   = Z
-    BIT,   // BIT test...................... | NV ...Z.              = A & M
-    BMI,   // Branch if Minus............... | .. .....         PC   = N
-    BNE,   // Branch if Not Equal........... | .. .....         PC   = !Z
-    BPL,   // Branch if Positive............ | .. .....         PC   = Z
-    BRK,   // BReaK......................... | .. B....       S PC   =
-    BVC,   // Branch if oVerflow Clear...... | .. .....         PC   = !V
-    BVS,   // Branch if oVerflow Set........ | .. .....         PC   = V
-    CLC,   // CLear Carry flag.............. | .. ....C              = 0
-    CLD,   // Clear Decimal Mode............ | .. .D...              = 0
-    CLI,   // Clear Interrupt Disable....... | .. ..I..              = 0
-    CLV,   // Clear oVerflow flag........... | .V .....              = 0
-    CMP,   // Compare....................... | N. ...ZC              = A - M
-    CPX,   // Compare X register............ | N. ...ZC              = X - M
-    CPY,   // Compare Y register............ | N. ...ZC              = Y - M
-    DEC,   // DECrement memory.............. | N. ...Z.            M = M - 1
-    DEX,   // DEcrement X register.......... | N. ...Z.   X          = X - 1
-    DEY,   // DEcrement Y register.......... | N. ...Z.     Y        = Y - 1
-    EOR,   // Exclusive OR (bitwise)........ | N. ...Z. A            = A ^ M
-    INC,   // INCrement memory.............. | N. ...Z.            M = M + 1
-    INX,   // INcrement X register.......... | N. ...Z.   X          = X + 1
-    INY,   // INcrement Y register.......... | N. ...Z.     Y        = Y + 1
-    JMP,   // JuMP.......................... | .. .....       S PC   =
-    JSR,   // Jump to SubRoutine............ | .. .....       S PC   =
-    LDA,   // LoaD Accumulator.............. | N. ...Z. A            = M
-    LDX,   // LoaD X register............... | N. ...Z.   X          = M
-    LDY,   // LoaD Y register............... | N. ...Z.     Y        = M
-    LSR,   // Logical Shift Right........... | N. ...ZC A            = A/2
-    //                               or N. ...ZC            M = M/2
-    NOP, // No OPeration.................. | .. .....              =
-    ORA, // inclusive OR (bitwise)........ | N. ...Z. A            = A | M
-    PHA, // PusH Accumulator.............. | .. .....       S    M = A
-    PHP, // PusH Processor status......... | .. .....       S    M = F
-    PLA, // PuLl Accumulator.............. | N. ...Z. A     S      = M (stack)
-    PLP, // PuLl Processor status......... | NV BDIZC       S      = M (stack)
-    ROL, // ROtate Left................... | N. ...ZC A            = C A rotated
-    //                               or N. ...ZC            M = C M rotated
-    ROR, // ROtate Right.................. | N. ...ZC A            = C A rotated
-    //                               or N. ...ZC            M = C M rotated
-    RTI,   // ReTurn from Interrupt......... | NV BDIZC         PC   = M (stack)
-    RTS,   // ReTurn from Subroutine........ | .. .....         PC   = M (stack)
-    SBC,   // SuBtract with Carry........... | NV ...ZC A            = A-M-(1-C)
-    SBCnd, // SuBtract with Carry........... | NV ...ZC A            = A-M-(1-C)
-    SEC,   // SEt Carry flag................ | .. ....C              = 1
-    SED,   // SEt Decimal flag.............. | .. .D...              = 1
-    SEI,   // SEt Interrupt disable......... | .. ..I..              = 1
-    STA,   // STore Accumulator............. | .. .....            M = A
-    STX,   // STore X register.............. | .. .....            M = X
-    STY,   // STore Y register.............. | .. .....            M = Y
-    TAX,   // Transfer Accumulator to X..... | N. ...Z.   X          = A
-    TAY,   // Transfer Accumulator to Y..... | N. ...Z.     Y        = A
-    TSX,   // Transfer Stack pointer to X... | N. ...Z.   X          = S
-    TXA,   // Transfer X to Accumulator..... | N. ...Z. A            = X
-    TXS,   // Transfer X to Stack pointer... | .. .....       S      = X
-    TYA,   // Transfer Y to Accumulator..... | N. ...Z. A            = Y
+    // ADd with Carry
+    ADC,
+
+    // ADd with Carry. This one has now decimal mode.
+    ADCnd,
+
+    // logical AND (bitwise)
+    AND,
+
+    // Arithmetic Shift Left
+    ASL,
+
+    // Branch if Carry Clear
+    BCC,
+
+    // Branch if Carry Set
+    BCS,
+
+    // Branch if Equal (to zero?)
+    BEQ,
+
+    // BIT test
+    BIT,
+
+    // Branch if Minus
+    BMI,
+
+    // Branch if Not Equal
+    BNE,
+
+    // Branch if Positive
+    BPL,
+
+    // Unconditional BRAnch
+    BRA,
+
+    // BReaK
+    BRK,
+
+    // BReaK, clearing decimal flag
+    BRKcld,
+
+    // Branch if oVerflow Clear
+    BVC,
+
+    // Branch if oVerflow Set
+    BVS,
+
+    // CLear Carry flag
+    CLC,
+
+    // Clear Decimal Mode
+    CLD,
+
+    // Clear Interrupt Disable
+    CLI,
+
+    // Clear oVerflow flag
+    CLV,
+
+    // Compare
+    CMP,
+
+    // Compare X register
+    CPX,
+
+    // Compare Y register
+    CPY,
+
+    // DECrement memory
+    DEC,
+
+    // DEcrement X register
+    DEX,
+
+    // DEcrement Y register
+    DEY,
+
+    // Exclusive OR (bitwise)
+    EOR,
+
+    // INCrement memory
+    INC,
+
+    // INcrement X register
+    INX,
+
+    // INcrement Y register
+    INY,
+
+    // JuMP
+    JMP,
+
+    // Jump to SubRoutine
+    JSR,
+
+    // LoaD Accumulator
+    LDA,
+
+    // LoaD X register
+    LDX,
+
+    // LoaD Y register
+    LDY,
+
+    // Logical Shift Right
+    LSR,
+
+    // No OPeration
+    NOP,
+
+    // inclusive OR (bitwise)
+    ORA,
+
+    // PusH Accumulator
+    PHA,
+
+    // PusH X
+    PHX,
+
+    // PusH Y
+    PHY,
+
+    // PusH Processor status
+    PHP,
+
+    // PuLl Accumulator
+    PLA,
+
+    // PuLl X
+    PLX,
+
+    // PuLl Y
+    PLY,
+
+    // PuLl Processor status
+    PLP,
+
+    // ROtate Left
+    ROL,
+
+    // ROtate Right
+    ROR,
+
+    // ReTurn from Interrupt
+    RTI,
+
+    // ReTurn from Subroutine
+    RTS,
+
+    // SuBtract with Carry
+    SBC,
+
+    // SuBtract with Carry. This one has now decimal mode.
+    SBCnd,
+
+    // SEt Carry flag
+    SEC,
+
+    // SEt Decimal flag
+    SED,
+
+    // SEt Interrupt disable
+    SEI,
+
+    // STore Accumulator
+    STA,
+
+    // STore X register
+    STX,
+
+    // STore Y register
+    STY,
+
+    // STore Zero
+    STZ,
+
+    // Transfer Accumulator to X
+    TAX,
+
+    // Transfer Accumulator to Y
+    TAY,
+
+    // Test and Reset Bits
+    TRB,
+
+    // Test and Set Bits
+    TSB,
+
+    // Transfer Stack pointer to X
+    TSX,
+
+    // Transfer X to Accumulator
+    TXA,
+
+    // Transfer X to Stack pointer
+    TXS,
+
+    // Transfer Y to Accumulator
+    TYA,
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub enum OpInput {
     UseImplied,
     UseImmediate(u8),
@@ -115,7 +237,7 @@ pub enum OpInput {
     UseAddress(u16),
 }
 
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub enum AddressingMode {
     // work directly on accumulator, e. g. `lsr a`.
     Accumulator,
@@ -158,10 +280,14 @@ pub enum AddressingMode {
 
     // load from (address stored at constant zero page address) plus Y register, e. g. `lda ($10),Y`.
     IndirectIndexedY,
+
+    // Address stored at constant zero page address
+    ZeroPageIndirect,
 }
 
 impl AddressingMode {
-    pub fn extra_bytes(self) -> u16 {
+    #[must_use]
+    pub const fn extra_bytes(self) -> u16 {
         match self {
             AddressingMode::Accumulator => 0,
             AddressingMode::Implied => 0,
@@ -177,6 +303,7 @@ impl AddressingMode {
             AddressingMode::BuggyIndirect => 2,
             AddressingMode::IndexedIndirectX => 1,
             AddressingMode::IndirectIndexedY => 1,
+            AddressingMode::ZeroPageIndirect => 1,
         }
     }
 }
@@ -184,6 +311,7 @@ impl AddressingMode {
 pub type DecodedInstr = (Instruction, OpInput);
 
 /// The NMOS 6502 variant. This one is present in the Commodore 64, early Apple IIs, etc.
+#[derive(Copy, Clone, Debug)]
 pub struct Nmos6502;
 
 impl crate::Variant for Nmos6502 {
@@ -449,59 +577,75 @@ impl crate::Variant for Nmos6502 {
     }
 }
 
-/// The Ricoh variant which has no decimal mode. This is what to use if you want to emulate the
-/// NES.
+/// The Ricoh variant which has no decimal mode. This is what to use if you want
+/// to emulate the NES.
+#[derive(Copy, Clone, Debug)]
 pub struct Ricoh2a03;
 
 impl crate::Variant for Ricoh2a03 {
     fn decode(opcode: u8) -> Option<(Instruction, AddressingMode)> {
-        match opcode {
-            0x61 => Some((Instruction::ADCnd, AddressingMode::IndexedIndirectX)),
-            0x65 => Some((Instruction::ADCnd, AddressingMode::ZeroPage)),
-            0x69 => Some((Instruction::ADCnd, AddressingMode::Immediate)),
-            0x6d => Some((Instruction::ADCnd, AddressingMode::Absolute)),
-            0x71 => Some((Instruction::ADCnd, AddressingMode::IndirectIndexedY)),
-            0x75 => Some((Instruction::ADCnd, AddressingMode::ZeroPageX)),
-            0x79 => Some((Instruction::ADCnd, AddressingMode::AbsoluteY)),
-            0x7d => Some((Instruction::ADCnd, AddressingMode::AbsoluteX)),
-            0xe1 => Some((Instruction::SBCnd, AddressingMode::IndexedIndirectX)),
-            0xe5 => Some((Instruction::SBCnd, AddressingMode::ZeroPage)),
-            0xe9 => Some((Instruction::SBCnd, AddressingMode::Immediate)),
-            0xed => Some((Instruction::SBCnd, AddressingMode::Absolute)),
-            0xf1 => Some((Instruction::SBCnd, AddressingMode::IndirectIndexedY)),
-            0xf5 => Some((Instruction::SBCnd, AddressingMode::ZeroPageX)),
-            0xf9 => Some((Instruction::SBCnd, AddressingMode::AbsoluteY)),
-            0xfd => Some((Instruction::SBCnd, AddressingMode::AbsoluteX)),
-            _ => Nmos6502::decode(opcode),
+        // It's the same as on NMOS, but doesn't support decimal mode.
+        match Nmos6502::decode(opcode) {
+            Some((Instruction::ADC, addressing_mode)) => {
+                Some((Instruction::ADCnd, addressing_mode))
+            }
+            Some((Instruction::SBC, addressing_mode)) => {
+                Some((Instruction::SBCnd, addressing_mode))
+            }
+            something_else => something_else,
         }
     }
 }
 
 /// Emulates some very early 6502s which have no ROR instruction. This one is used in very early
 /// KIM-1s.
+#[derive(Copy, Clone, Debug)]
 pub struct RevisionA;
 
 impl crate::Variant for RevisionA {
     fn decode(opcode: u8) -> Option<(Instruction, AddressingMode)> {
-        match opcode {
-            0x66 => None,
-            0x6a => None,
-            0x6e => None,
-            0x76 => None,
-            0x7e => None,
-            _ => Nmos6502::decode(opcode),
+        // It's the same as on NMOS, but has no ROR instruction.
+        match Nmos6502::decode(opcode) {
+            Some((Instruction::ROR, _)) => None,
+            something_else => something_else,
         }
     }
 }
 
 /// Emulates the 65C02, which has a few bugfixes, and another addressing mode
+#[derive(Copy, Clone, Debug)]
 pub struct Cmos6502;
 
 impl crate::Variant for Cmos6502 {
     fn decode(opcode: u8) -> Option<(Instruction, AddressingMode)> {
         // TODO: We obviously need to add the other CMOS instructions here.
         match opcode {
+            0x00 => Some((Instruction::BRKcld, AddressingMode::Implied)),
+            0x1a => Some((Instruction::INC, AddressingMode::Accumulator)),
+            0x3a => Some((Instruction::DEC, AddressingMode::Accumulator)),
             0x6c => Some((Instruction::JMP, AddressingMode::Indirect)),
+            0x80 => Some((Instruction::BRA, AddressingMode::Relative)),
+            0x64 => Some((Instruction::STZ, AddressingMode::ZeroPage)),
+            0x74 => Some((Instruction::STZ, AddressingMode::ZeroPageX)),
+            0x9c => Some((Instruction::STZ, AddressingMode::Absolute)),
+            0x9e => Some((Instruction::STZ, AddressingMode::AbsoluteX)),
+            0x7a => Some((Instruction::PLY, AddressingMode::Implied)),
+            0xfa => Some((Instruction::PLX, AddressingMode::Implied)),
+            0x5a => Some((Instruction::PHY, AddressingMode::Implied)),
+            0xda => Some((Instruction::PHX, AddressingMode::Implied)),
+            0x04 => Some((Instruction::TSB, AddressingMode::ZeroPage)),
+            0x14 => Some((Instruction::TRB, AddressingMode::ZeroPage)),
+            0x0c => Some((Instruction::TSB, AddressingMode::Absolute)),
+            0x1c => Some((Instruction::TRB, AddressingMode::Absolute)),
+            0x12 => Some((Instruction::ORA, AddressingMode::ZeroPageIndirect)),
+            0x32 => Some((Instruction::AND, AddressingMode::ZeroPageIndirect)),
+            0x52 => Some((Instruction::EOR, AddressingMode::ZeroPageIndirect)),
+            0x72 => Some((Instruction::ADC, AddressingMode::ZeroPageIndirect)),
+            0x92 => Some((Instruction::STA, AddressingMode::ZeroPageIndirect)),
+            0xb2 => Some((Instruction::LDA, AddressingMode::ZeroPageIndirect)),
+            0xd2 => Some((Instruction::CMP, AddressingMode::ZeroPageIndirect)),
+            0xf2 => Some((Instruction::SBC, AddressingMode::ZeroPageIndirect)),
+            0x89 => Some((Instruction::BIT, AddressingMode::Immediate)),
             _ => Nmos6502::decode(opcode),
         }
     }

src/lib.rs

@@ -25,15 +25,29 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
+#![warn(clippy::all, clippy::pedantic)]
+#![warn(
+    absolute_paths_not_starting_with_crate,
+    rustdoc::invalid_html_tags,
+    missing_copy_implementations,
+    missing_debug_implementations,
+    semicolon_in_expressions_from_macros,
+    unreachable_pub,
+    unused_crate_dependencies,
+    unused_extern_crates,
+    variant_size_differences,
+    clippy::missing_const_for_fn
+)]
+#![deny(anonymous_parameters, macro_use_extern_crate, pointer_structural_match)]
+#![allow(clippy::module_name_repetitions)]
+// Registers and ops follow the 6502 naming convention and have similar names at
+// times
+#![allow(clippy::similar_names)]
+#![allow(clippy::match_same_arms)]
+#![allow(clippy::too_many_lines)]
 #![no_std]
 
 #[doc = include_str!("../README.md")]
-#[macro_use]
-extern crate log;
-
-#[macro_use]
-extern crate bitflags;
-
 pub mod cpu;
 pub mod instruction;
 pub mod memory;

src/memory.rs

@@ -47,7 +47,7 @@ pub const IRQ_INTERRUPT_VECTOR_HI: u16 = 0xFFFF;
 const MEMORY_SIZE: usize = (ADDR_HI_BARE - ADDR_LO_BARE) as usize + 1usize;
 
 // FIXME: Should this use indirection for `bytes`?
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
 pub struct Memory {
     bytes: [u8; MEMORY_SIZE],
 }
@@ -58,10 +58,37 @@ impl Default for Memory {
     }
 }
 
+/// Trait for a bus that can read and write bytes.
+///
+/// This is used to abstract the memory and I/O operations of the CPU.
+///
+/// # Examples
+///
+/// ```
+/// use mos6502::memory::{Bus, Memory};
+///
+/// let mut memory = Memory::new();
+/// memory.set_byte(0x0000, 0x12);
+/// assert_eq!(memory.get_byte(0x0000), 0x12);
+/// ```
 pub trait Bus {
+    /// Returns the byte at the given address.
     fn get_byte(&mut self, address: u16) -> u8;
+
+    /// Sets the byte at the given address to the given value.
     fn set_byte(&mut self, address: u16, value: u8);
 
+    /// Sets the bytes starting at the given address to the given values.
+    ///
+    /// This is a default implementation that calls `set_byte` for each byte.
+    ///
+    /// # Note
+    ///
+    /// This assumes that the length of `values` is less than or equal to
+    /// [`u16::MAX`] (65535). If the length of `values` is greater than `u16::MAX`,
+    /// this will truncate the length. This assumption is made because the
+    /// maximum addressable memory for the 6502 is 64KB.
+    #[allow(clippy::cast_possible_truncation)]
     fn set_bytes(&mut self, start: u16, values: &[u8]) {
         for i in 0..values.len() as u16 {
             self.set_byte(start + i, values[i as usize]);
@@ -70,7 +97,8 @@ pub trait Bus {
 }
 
 impl Memory {
-    pub fn new() -> Memory {
+    #[must_use]
+    pub const fn new() -> Memory {
         Memory {
             bytes: [0; MEMORY_SIZE],
         }
@@ -82,12 +110,14 @@ impl Bus for Memory {
         self.bytes[address as usize]
     }
 
-    // Sets the byte at the given address to the given value and returns the
-    // previous value at the address.
+    /// Sets the byte at the given address to the given value and returns the
+    /// previous value at the address.
     fn set_byte(&mut self, address: u16, value: u8) {
         self.bytes[address as usize] = value;
     }
 
+    /// Fast way to set multiple bytes in memory when the underlying memory is a
+    /// consecutive block of bytes.
     fn set_bytes(&mut self, start: u16, values: &[u8]) {
         let start = start as usize;
 
@@ -103,7 +133,7 @@ mod tests {
     use super::*;
 
     #[test]
-    #[should_panic]
+    #[should_panic(expected = "range end index 65537 out of range for slice of length 65536")]
     fn test_memory_overflow_panic() {
         let mut memory = Memory::new();
         memory.set_bytes(0xFFFE, &[1, 2, 3]);

src/registers.rs

@@ -25,8 +25,11 @@
 // ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 // POSSIBILITY OF SUCH DAMAGE.
 
+use bitflags::bitflags;
+
 // Useful for constructing Status instances
-#[derive(Copy, Clone)]
+#[derive(Copy, Clone, Debug)]
+#[allow(clippy::struct_excessive_bools)]
 pub struct StatusArgs {
     pub negative: bool,
     pub overflow: bool,
@@ -39,7 +42,8 @@ pub struct StatusArgs {
 }
 
 impl StatusArgs {
-    pub fn none() -> StatusArgs {
+    #[must_use]
+    pub const fn none() -> StatusArgs {
         StatusArgs {
             negative: false,
             overflow: false,
@@ -71,6 +75,7 @@ bitflags! {
 }
 
 impl Status {
+    #[must_use]
     pub fn new(
         StatusArgs {
             negative,
@@ -86,28 +91,28 @@ impl Status {
         let mut out = Status::empty();
 
         if negative {
-            out |= Status::PS_NEGATIVE
+            out |= Status::PS_NEGATIVE;
         }
         if overflow {
-            out |= Status::PS_OVERFLOW
+            out |= Status::PS_OVERFLOW;
         }
         if unused {
-            out |= Status::PS_UNUSED
+            out |= Status::PS_UNUSED;
         }
         if brk {
-            out |= Status::PS_BRK
+            out |= Status::PS_BRK;
         }
         if decimal_mode {
-            out |= Status::PS_DECIMAL_MODE
+            out |= Status::PS_DECIMAL_MODE;
         }
         if disable_interrupts {
-            out |= Status::PS_DISABLE_INTERRUPTS
+            out |= Status::PS_DISABLE_INTERRUPTS;
         }
         if zero {
-            out |= Status::PS_ZERO
+            out |= Status::PS_ZERO;
         }
         if carry {
-            out |= Status::PS_CARRY
+            out |= Status::PS_CARRY;
         }
 
         out
@@ -146,7 +151,8 @@ impl Default for Status {
 pub struct StackPointer(pub u8);
 
 impl StackPointer {
-    pub fn to_u16(self) -> u16 {
+    #[must_use]
+    pub const fn to_u16(self) -> u16 {
         let StackPointer(val) = self;
         u16::from_le_bytes([val, 0x01])
     }
@@ -177,6 +183,7 @@ impl Default for Registers {
 }
 
 impl Registers {
+    #[must_use]
     pub fn new() -> Registers {
         // TODO akeeton: Revisit these defaults.
         Registers {