Code Cleanup

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

src/cpu.rs

@@ -31,10 +31,8 @@ use crate::Variant;
 
 use crate::registers::{Registers, StackPointer, Status, StatusArgs};
 
-fn arr_to_addr(arr: &[u8]) -> u16 {
-    debug_assert!(arr.len() == 2);
-
-    u16::from(arr[0]) + (u16::from(arr[1]) << 8usize)
+fn address_from_bytes(lo: u8, hi: u8) -> u16 {
+    u16::from(lo) + (u16::from(hi) << 8usize)
 }
 
 #[derive(Clone)]
@@ -49,6 +47,9 @@ 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 {
@@ -144,24 +145,52 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                     AddressingMode::Absolute => {
                         // Use [u8, ..2] from instruction as address
                         // (Output: a 16-bit address)
-                        OpInput::UseAddress(arr_to_addr(&slice))
+                        OpInput::UseAddress(address_from_bytes(slice[0], slice[1]))
                     }
                     AddressingMode::AbsoluteX => {
                         // Use [u8, ..2] from instruction as address, add X
                         // (Output: a 16-bit address)
-                        OpInput::UseAddress(arr_to_addr(&slice).wrapping_add(x.into()))
+                        OpInput::UseAddress(
+                            address_from_bytes(slice[0], slice[1]).wrapping_add(x.into()),
+                        )
                     }
                     AddressingMode::AbsoluteY => {
                         // Use [u8, ..2] from instruction as address, add Y
                         // (Output: a 16-bit address)
-                        OpInput::UseAddress(arr_to_addr(&slice).wrapping_add(y.into()))
+                        OpInput::UseAddress(
+                            address_from_bytes(slice[0], slice[1]).wrapping_add(y.into()),
+                        )
                     }
                     AddressingMode::Indirect => {
                         // Use [u8, ..2] from instruction as an address. Interpret the
                         // two bytes starting at that address as an address.
                         // (Output: a 16-bit address)
-                        let slice = read_address(memory, arr_to_addr(&slice));
-                        OpInput::UseAddress(arr_to_addr(&slice))
+                        // TODO: If the pointer ends in 0xff, then incrementing it would propagate
+                        // the carry to the high byte of the pointer. This incurs a cost of one
+                        // machine cycle on the real 65C02, which is not implemented here.
+                        let slice = read_address(memory, address_from_bytes(slice[0], slice[1]));
+                        OpInput::UseAddress(address_from_bytes(slice[0], slice[1]))
+                    }
+                    AddressingMode::BuggyIndirect => {
+                        // Use [u8, ..2] from instruction as an address. Interpret the
+                        // two bytes starting at that address as an address.
+                        // (Output: a 16-bit address)
+                        let pointer = address_from_bytes(slice[0], slice[1]);
+
+                        let low_byte_of_target = memory.get_byte(pointer);
+
+                        let low_byte_of_incremented_pointer =
+                            pointer.to_le_bytes()[0].wrapping_add(1);
+                        let incremented_pointer = u16::from_le_bytes([
+                            low_byte_of_incremented_pointer,
+                            pointer.to_le_bytes()[1],
+                        ]);
+
+                        let high_byte_of_target = memory.get_byte(incremented_pointer);
+                        OpInput::UseAddress(address_from_bytes(
+                            low_byte_of_target,
+                            high_byte_of_target,
+                        ))
                     }
                     AddressingMode::IndexedIndirectX => {
                         // Use [u8, ..1] from instruction
@@ -170,7 +199,7 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                         // (Output: a 16-bit address)
                         let start = slice[0].wrapping_add(x);
                         let slice = read_address(memory, u16::from(start));
-                        OpInput::UseAddress(arr_to_addr(&slice))
+                        OpInput::UseAddress(address_from_bytes(slice[0], slice[1]))
                     }
                     AddressingMode::IndirectIndexedY => {
                         // Use [u8, ..1] from instruction
@@ -179,7 +208,9 @@ impl<M: Bus, V: Variant> CPU<M, V> {
                         // (Output: a 16-bit address)
                         let start = slice[0];
                         let slice = read_address(memory, u16::from(start));
-                        OpInput::UseAddress(arr_to_addr(&slice).wrapping_add(y.into()))
+                        OpInput::UseAddress(
+                            address_from_bytes(slice[0], slice[1]).wrapping_add(y.into()),
+                        )
                     }
                 };
 
@@ -610,18 +641,29 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         }
     }
 
+    /// 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 = Self::value_is_negative(value);
 
         status.set_with_mask(
             Status::PS_ZERO | Status::PS_NEGATIVE,
             Status::new(StatusArgs {
                 zero: is_zero,
-                negative: Self::value_is_negative(value),
+                negative: is_negative,
                 ..StatusArgs::none()
             }),
         );
@@ -923,12 +965,13 @@ impl<M: Bus, V: Variant> CPU<M, V> {
         *val = value_new;
 
         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: Self::value_is_negative(value_new),
                 zero: is_zero,
+                negative: is_negative,
                 ..StatusArgs::none()
             }),
         );
@@ -994,20 +1037,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);
@@ -1073,6 +1120,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;
@@ -1176,6 +1228,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

@@ -117,21 +117,47 @@ pub enum OpInput {
 
 #[derive(Copy, Clone, Debug)]
 pub enum AddressingMode {
-    Accumulator,      // 1    LSR A        work directly on accumulator
-    Implied,          // 1    BRK
-    Immediate,        // 2    LDA #10      8-bit constant in instruction
-    ZeroPage,         // 2    LDA $00      zero-page address
-    ZeroPageX,        // 2    LDA $80,X    address is X register + 8-bit constant
-    ZeroPageY,        // 2    LDX $10,Y    address is Y register + 8-bit constant
-    Relative,         // 2    BNE LABEL    branch target as signed relative offset
-    Absolute,         // 3    JMP $1000    full 16-bit address
-    AbsoluteX,        // 3    STA $1000,X  full 16-bit address plus X register
-    AbsoluteY,        // 3    STA $1000,Y  full 16-bit address plus Y register
-    Indirect,         // 3    JMP ($1000)  jump to address stored at address
-    IndexedIndirectX, // 2    LDA ($10,X)  load from address stored at (constant
-    //                   zero page address plus X register)
-    IndirectIndexedY, // 2    LDA ($10),Y  load from (address stored at constant
-                      //                   zero page address) plus Y register
+    // work directly on accumulator, e. g. `lsr a`.
+    Accumulator,
+
+    // BRK
+    Implied,
+
+    // 8-bit constant in instruction, e. g. `lda #10`.
+    Immediate,
+
+    // zero-page address, e. g. `lda $00`.
+    ZeroPage,
+
+    // address is X register + 8-bit constant, e. g. `lda $80,x`.
+    ZeroPageX,
+
+    // address is Y register + 8-bit constant, e. g. `ldx $10,y`.
+    ZeroPageY,
+
+    // branch target as signed relative offset, e. g. `bne label`.
+    Relative,
+
+    // full 16-bit address, e. g. `jmp $1000`.
+    Absolute,
+
+    // full 16-bit address plus X register, e. g. `sta $1000,X`.
+    AbsoluteX,
+
+    // full 16-bit address plus Y register, e. g. `sta $1000,Y`.
+    AbsoluteY,
+
+    // jump to address stored at address, with the page-crossing bug found in NMOS chips, e. g. `jmp ($1000)`.
+    BuggyIndirect,
+
+    // jump to address stored at address, e. g. `jmp ($1000)`.
+    Indirect,
+
+    // load from address stored at (constant zero page address plus X register), e. g. `lda ($10,X)`.
+    IndexedIndirectX,
+
+    // load from (address stored at constant zero page address) plus Y register, e. g. `lda ($10),Y`.
+    IndirectIndexedY,
 }
 
 impl AddressingMode {
@@ -149,6 +175,7 @@ impl AddressingMode {
             AddressingMode::AbsoluteX => 2,
             AddressingMode::AbsoluteY => 2,
             AddressingMode::Indirect => 2,
+            AddressingMode::BuggyIndirect => 2,
             AddressingMode::IndexedIndirectX => 1,
             AddressingMode::IndirectIndexedY => 1,
         }
@@ -272,7 +299,7 @@ impl crate::Variant for Nmos6502 {
             0x69 => Some((Instruction::ADC, AddressingMode::Immediate)),
             0x6a => Some((Instruction::ROR, AddressingMode::Accumulator)),
             0x6b => None,
-            0x6c => Some((Instruction::JMP, AddressingMode::Indirect)),
+            0x6c => Some((Instruction::JMP, AddressingMode::BuggyIndirect)),
             0x6d => Some((Instruction::ADC, AddressingMode::Absolute)),
             0x6e => Some((Instruction::ROR, AddressingMode::Absolute)),
             0x6f => None,
@@ -471,3 +498,17 @@ impl crate::Variant for RevisionA {
         }
     }
 }
+
+/// 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 {
+            0x6c => Some((Instruction::JMP, AddressingMode::Indirect)),
+            _ => Nmos6502::decode(opcode),
+        }
+    }
+}

src/memory.rs

@@ -81,6 +81,14 @@ pub trait Bus {
     /// 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]);
@@ -102,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;