Mostly fix label and number parsing

Author: ian-h-chamberlain

src/data/redcode.pest

@@ -2,7 +2,7 @@ WHITESPACE = _{ " " | "\t" }
 
 COMMENT = @{ ";" ~ (!NEWLINE ~ ANY)* ~ &NEWLINE }
 
-RedcodeFile = { SOI ~ (Line ~ NEWLINE)+ ~ EOI }
+RedcodeFile = { SOI ~ (Line ~ NEWLINE)+ ~ Line? ~ EOI }
 
 Line = { LabelDeclaration* ~ Instruction? }
 
@@ -28,7 +28,7 @@ AddressMode = { "#" | "$" | "*" | "@" | "{" | "<" | "}" | ">" }
 
 Expr = { Number | Label }
 
-Number = @{ ASCII_DIGIT+ }
+Number = @{ ("-" | "+")? ~ ASCII_DIGIT+ }
 
 Alpha = _{ ASCII_ALPHA | "_" }
 

src/load_file.rs

@@ -94,7 +94,7 @@ enum_string!(pub AddressMode {
 
 impl Default for AddressMode {
     fn default() -> Self {
-        Self::Immediate
+        Self::Direct
     }
 }
 
@@ -119,12 +119,21 @@ impl ToString for Value {
     }
 }
 
-#[derive(Clone, Default, Debug, PartialEq)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct Field {
     pub address_mode: AddressMode,
     pub value: Value,
 }
 
+impl Default for Field {
+    fn default() -> Self {
+        Self {
+            address_mode: AddressMode::Immediate,
+            value: Value::default(),
+        }
+    }
+}
+
 impl Field {
     pub fn direct(value: i32) -> Self {
         Self {
@@ -151,25 +160,14 @@ impl ToString for Field {
     }
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Clone, Debug, Default, PartialEq)]
 pub struct Instruction {
     pub opcode: Opcode,
     pub modifier: Modifier,
     pub field_a: Field,
     pub field_b: Field,
 }
 
-impl Default for Instruction {
-    fn default() -> Self {
-        Instruction {
-            opcode: Opcode::default(),
-            modifier: Modifier::default(),
-            field_a: Field::direct(0),
-            field_b: Field::direct(0),
-        }
-    }
-}
-
 impl Instruction {
     pub fn new(opcode: Opcode, field_a: Field, field_b: Field) -> Self {
         let modifier =
@@ -218,10 +216,9 @@ impl Core {
         self.instructions[index] = value;
     }
 
-    pub fn add_labels<L>(&mut self, index: usize, labels: L) -> Result<(), String>
+    pub fn add_labels<T>(&mut self, index: usize, labels: Vec<T>) -> Result<(), String>
     where
-        L: IntoIterator,
-        L::Item: Into<String>,
+        T: Into<String>,
     {
         if index > self.instructions.len() {
             return Err(format!(
@@ -277,7 +274,7 @@ impl fmt::Debug for Core {
     fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
         write!(
             formatter,
-            "Labels:{:?}\nCore:\n{}",
+            "Labels: {:?}\nCore:\n{}",
             self.labels,
             self.dump()
         )
@@ -297,11 +294,11 @@ mod tests {
             opcode: Opcode::Dat,
             modifier: Modifier::F,
             field_a: Field {
-                address_mode: AddressMode::Direct,
+                address_mode: AddressMode::Immediate,
                 value: Value::Literal(0),
             },
             field_b: Field {
-                address_mode: AddressMode::Direct,
+                address_mode: AddressMode::Immediate,
                 value: Value::Literal(0),
             },
         };
@@ -430,20 +427,18 @@ mod tests {
     fn labels() {
         let mut core = Core::new(200);
 
-        core.add_labels(0, vec!["foo", "bar"]).unwrap();
-
-        core.add_labels(123, vec!["baz", "boo"]).unwrap();
+        core.add_labels(123, vec!["baz"]).expect("Should add label");
+        core.add_labels(0, vec!["foo", "bar"])
+            .expect("Should add two labels");
 
         core.add_labels(256, vec!["goblin"])
-            .expect_err("Should have failed to add labels for 256, but didn't");
-
+            .expect_err("Should fail to add labels > 200");
         core.add_labels(5, vec!["baz"])
-            .expect_err("Should have failed to add duplicate label");
+            .expect_err("Should fail to add duplicate label");
 
         assert_eq!(core.label_address("foo").unwrap(), 0);
         assert_eq!(core.label_address("bar").unwrap(), 0);
         assert_eq!(core.label_address("baz").unwrap(), 123);
-        assert_eq!(core.label_address("boo").unwrap(), 123);
 
         assert!(core.label_address("goblin").is_none());
         assert!(core.label_address("never_mentioned").is_none());

src/parser.rs

@@ -75,32 +75,33 @@ pub fn parse(file_contents: &str) -> Result<Core, Error> {
         .next()
         .ok_or_else(Error::no_input)?;
 
-    for (i, mut line_pairs) in parse_result
+    let mut i = 0;
+    for mut line_pair in parse_result
         .into_inner()
         .map(Pair::into_inner)
         .filter(|line_pair| line_pair.peek().is_some())
-        .enumerate()
     {
-        let label_pairs = line_pairs
+        let label_pairs = line_pair
             .take_while_ref(|pair| pair.as_rule() == Rule::Label)
             .map(|pair| pair.as_str().to_owned());
 
-        core.add_labels(i, label_pairs)?;
+        core.add_labels(i, label_pairs.collect())?;
 
-        core.set(i, parse_instruction(line_pairs));
+        if line_pair.peek().is_some() {
+            core.set(i, parse_instruction(line_pair));
+            i += 1;
+        }
     }
 
     Ok(core)
 }
 
 fn parse_instruction(mut instruction_pairs: Pairs<Rule>) -> Instruction {
-    dbg!(&instruction_pairs.peek());
     let mut operation_pairs = instruction_pairs
         .next()
         .expect("Operation must be first pair after Label in Instruction")
         .into_inner();
 
-    dbg!(&operation_pairs.peek());
     let opcode = parse_opcode(
         &operation_pairs
             .next()
@@ -167,10 +168,18 @@ fn parse_field(field_pair: Pair<Rule>) -> Field {
 }
 
 fn parse_value(value_pair: Pair<Rule>) -> Value {
-    if value_pair.as_rule() == Rule::Number {
-        Value::Literal(i32::from_str_radix(value_pair.as_str(), 10).unwrap())
-    } else {
-        Value::Label(value_pair.as_str().to_owned())
+    let expr_inner = value_pair
+        .into_inner()
+        .next()
+        .expect("Expr must have inner value");
+
+    match expr_inner.as_rule() {
+        Rule::Number => Value::Literal(
+            i32::from_str_radix(expr_inner.as_str(), 10)
+                .expect("Number type must be decimal integer"),
+        ),
+        Rule::Label => Value::Label(expr_inner.as_str().to_owned()),
+        _ => unreachable!(),
     }
 }
 
@@ -306,15 +315,27 @@ mod tests {
         }
     }
 
+    #[test]
+    fn duplicate_labels() {
+        let simple_input = "
+            label1  dat 0,0
+            label1  dat 0,0
+        ";
+
+        parse(simple_input).expect_err("Should fail for duplicate label");
+    }
+
     #[test]
     fn parse_simple_file() {
         let simple_input = "
             preload
             begin:  mov 1, 3 ; make sure comments parse out
                     mov 100, #12
             loop:
-            main    dat 0, 0
-                    jmp 123, 45
+            main    dat #0, #0
+                    jmp +123, #45
+                    jmp begin
+                    jmp -1
         ";
 
         let mut expected_core = Core::default();
@@ -333,14 +354,31 @@ mod tests {
         );
         expected_core.set(
             3,
-            Instruction::new(Opcode::Jmp, Field::direct(123), Field::direct(45)),
+            Instruction::new(Opcode::Jmp, Field::direct(123), Field::immediate(45)),
+        );
+        expected_core.set(
+            4,
+            Instruction::new(
+                Opcode::Jmp,
+                Field {
+                    address_mode: AddressMode::Direct,
+                    value: Value::Label(String::from("begin")),
+                },
+                Field::immediate(0),
+            ),
+        );
+        expected_core.set(
+            5,
+            Instruction::new(Opcode::Jmp, Field::direct(-1), Field::immediate(0)),
         );
 
         expected_core
             .add_labels(0, vec!["preload", "begin"])
             .unwrap();
         expected_core.add_labels(2, vec!["loop", "main"]).unwrap();
 
-        assert_eq!(parse(simple_input).unwrap(), expected_core);
+        let parsed = parse(simple_input).unwrap();
+
+        assert_eq!(parsed, expected_core);
     }
 }

tests/dump.rs

@@ -13,6 +13,7 @@ fn run_test(input: &str, expected_output: &str) {
 }
 
 #[test]
+#[ignore = "Fails because labels are not yet converted to offsets"]
 fn dump_all_opcodes() {
     run_test(
         include_str!("data/test.red"),
@@ -21,7 +22,7 @@ fn dump_all_opcodes() {
 }
 
 #[test]
-#[ignore] // Until we have support for keeping metadata comments, EQU, etc.
+#[ignore = "Fails for metadata comments, EQU, etc."]
 fn dump_icws94_example_dwarf() {
     run_test(
         include_str!("data/dwarf.red"),