From 560eb1ca75dbebc52ec55b1af0ddab5c397eb812 Mon Sep 17 00:00:00 2001 From: RandomSearch <101704343+RandomSearch18@users.noreply.github.com> Date: Tue, 11 Nov 2025 12:06:10 +0000 Subject: [PATCH 1/5] Include line numbers on assembler errors --- demos/tests/invalid.lmc | 1 + src/rmc_assemble.rs | 62 ++++++++++++++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 13 deletions(-) create mode 100644 demos/tests/invalid.lmc diff --git a/demos/tests/invalid.lmc b/demos/tests/invalid.lmc new file mode 100644 index 0000000..f9ceacc --- /dev/null +++ b/demos/tests/invalid.lmc @@ -0,0 +1 @@ +ABC 1 \ No newline at end of file diff --git a/src/rmc_assemble.rs b/src/rmc_assemble.rs index b2905ee..01e3060 100644 --- a/src/rmc_assemble.rs +++ b/src/rmc_assemble.rs @@ -1,7 +1,5 @@ use clap::Parser; -use std::{ - collections::HashMap, fs, path::PathBuf -}; +use std::{collections::HashMap, fmt, fs, path::PathBuf}; use rusty_man_computer::value::Value; @@ -38,11 +36,36 @@ enum Line { } #[derive(Debug)] -enum ParseError { +enum ParseErrorType { InvalidOpcode(String), OperandOutOfRange(i16), } +impl fmt::Display for ParseErrorType { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + ParseErrorType::InvalidOpcode(opcode) => { + write!(f, "Invalid opcode: {}", opcode) + } + ParseErrorType::OperandOutOfRange(value) => { + write!(f, "Operand out of range: {}", value) + } + } + } +} + +#[derive(Debug)] +struct ParseError { + error: ParseErrorType, + line: usize, +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Parse error on line {}: {}", self.line, self.error) + } +} + fn parse_opcode(string: &str) -> Option { match string { "HLT" => Some(Opcode::HLT), @@ -64,8 +87,10 @@ fn parse_opcode(string: &str) -> Option { fn parse_assembly(program: &str) -> Vec> { program .lines() - .map(|line| { + .enumerate() + .map(|(line_index, line)| { let line = line.trim(); + let line_number = line_index + 1; if line.is_empty() || line.starts_with("//") { return Ok(Line::Empty); } @@ -87,11 +112,21 @@ fn parse_assembly(program: &str) -> Vec> { let string = match parts.get(1) { Some(string) => string, // This means there's only one part: there's nothing to label, so it's just an invalid opcode - None => return Err(ParseError::InvalidOpcode(parts[0].to_string())), + None => { + return Err(ParseError { + error: ParseErrorType::InvalidOpcode(parts[0].to_string()), + line: line_number, + }); + } }; match parse_opcode(string) { Some(opcode) => opcode, - None => return Err(ParseError::InvalidOpcode(string.to_string())), + None => { + return Err(ParseError { + error: ParseErrorType::InvalidOpcode(string.to_string()), + line: line_number, + }); + } } } }; @@ -106,7 +141,10 @@ fn parse_assembly(program: &str) -> Vec> { Some(string) => match string.parse::() { Ok(value) => Some(Operand::Value( // If the number doesn't fit within a Value, return an OperandOutOfRange error - Value::new(value).map_err(|_| ParseError::OperandOutOfRange(value))?, + Value::new(value).map_err(|_| ParseError { + error: ParseErrorType::OperandOutOfRange(value), + line: line_number, + })?, )), Err(_) => Some(Operand::Label(string.to_string())), }, @@ -222,17 +260,15 @@ fn main() -> Result<(), String> { match assembler_result { Err(error) => match error { AssemblerError::ParseError(parse_error) => { - return Err(format!("Parse error: {:?}", parse_error)); + return Err(parse_error.to_string()); } AssemblerError::MachineCodeError(message) => { return Err(message.to_string()); } }, Ok(machine_code) => { - let machine_code_bytes: Vec = machine_code - .iter() - .flat_map(|&i| i.to_be_bytes()) - .collect(); + let machine_code_bytes: Vec = + machine_code.iter().flat_map(|&i| i.to_be_bytes()).collect(); fs::write(args.output, machine_code_bytes) .map_err(|e| format!("Failed to write output file: {}", e)) } From 9d2ad082a7b44d32c444bec3857cc4e59cedca02 Mon Sep 17 00:00:00 2001 From: RandomSearch <101704343+RandomSearch18@users.noreply.github.com> Date: Tue, 11 Nov 2025 12:16:06 +0000 Subject: [PATCH 2/5] Use .ok_or() instead of match statements --- src/rmc_assemble.rs | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/rmc_assemble.rs b/src/rmc_assemble.rs index 01e3060..78bedb5 100644 --- a/src/rmc_assemble.rs +++ b/src/rmc_assemble.rs @@ -109,25 +109,14 @@ fn parse_assembly(program: &str) -> Vec> { let opcode = match first_part_as_opcode { Some(opcode) => opcode, None => { - let string = match parts.get(1) { - Some(string) => string, - // This means there's only one part: there's nothing to label, so it's just an invalid opcode - None => { - return Err(ParseError { - error: ParseErrorType::InvalidOpcode(parts[0].to_string()), - line: line_number, - }); - } - }; - match parse_opcode(string) { - Some(opcode) => opcode, - None => { - return Err(ParseError { - error: ParseErrorType::InvalidOpcode(string.to_string()), - line: line_number, - }); - } - } + let string = parts.get(1).ok_or(ParseError { + error: ParseErrorType::InvalidOpcode(parts[0].to_string()), + line: line_number, + })?; + parse_opcode(string).ok_or(ParseError { + error: ParseErrorType::InvalidOpcode(string.to_string()), + line: line_number, + })? } }; let operand_part = if label.is_some() { From d8357ac289ac36037b6b8586e06f1a5ea9d51f8c Mon Sep 17 00:00:00 2001 From: RandomSearch <101704343+RandomSearch18@users.noreply.github.com> Date: Tue, 11 Nov 2025 12:39:44 +0000 Subject: [PATCH 3/5] Return AssemblerErrors from main() instead of Strings Still not a massive fan of how we're handling errors but it's okay --- src/rmc_assemble.rs | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/rmc_assemble.rs b/src/rmc_assemble.rs index 78bedb5..2036795 100644 --- a/src/rmc_assemble.rs +++ b/src/rmc_assemble.rs @@ -1,5 +1,5 @@ use clap::Parser; -use std::{collections::HashMap, fmt, fs, path::PathBuf}; +use std::{collections::HashMap, fmt, fs, io, path::PathBuf}; use rusty_man_computer::value::Value; @@ -206,10 +206,20 @@ fn generate_machine_code(lines: Vec) -> Result, &'static str> { Ok(output) } -#[derive(Debug)] enum AssemblerError { ParseError(ParseError), MachineCodeError(&'static str), + WriteError(io::Error), +} + +impl fmt::Debug for AssemblerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + AssemblerError::ParseError(e) => write!(f, "{}", e), + AssemblerError::MachineCodeError(e) => write!(f, "Machine code error: {}", e), + AssemblerError::WriteError(e) => write!(f, "Write error: {}", e), + } + } } fn assemble(program: &str) -> Result, AssemblerError> { @@ -242,24 +252,16 @@ pub struct Args { output: PathBuf, } -fn main() -> Result<(), String> { +fn main() -> Result<(), AssemblerError> { let args = Args::parse(); let program = std::fs::read_to_string(args.program).expect("Failed to read program"); let assembler_result = assemble(&program); match assembler_result { - Err(error) => match error { - AssemblerError::ParseError(parse_error) => { - return Err(parse_error.to_string()); - } - AssemblerError::MachineCodeError(message) => { - return Err(message.to_string()); - } - }, + Err(error) => Err(error), Ok(machine_code) => { let machine_code_bytes: Vec = machine_code.iter().flat_map(|&i| i.to_be_bytes()).collect(); - fs::write(args.output, machine_code_bytes) - .map_err(|e| format!("Failed to write output file: {}", e)) + fs::write(args.output, machine_code_bytes).map_err(|e| AssemblerError::WriteError(e)) } } } From a30591fb16bce49b6641725f03df3080013ef9a5 Mon Sep 17 00:00:00 2001 From: RandomSearch <101704343+RandomSearch18@users.noreply.github.com> Date: Tue, 18 Nov 2025 11:22:51 +0000 Subject: [PATCH 4/5] Handle read errors instead of panicking --- src/rmc_assemble.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/rmc_assemble.rs b/src/rmc_assemble.rs index 2036795..1065003 100644 --- a/src/rmc_assemble.rs +++ b/src/rmc_assemble.rs @@ -209,6 +209,7 @@ fn generate_machine_code(lines: Vec) -> Result, &'static str> { enum AssemblerError { ParseError(ParseError), MachineCodeError(&'static str), + ReadError(io::Error), WriteError(io::Error), } @@ -218,6 +219,7 @@ impl fmt::Debug for AssemblerError { AssemblerError::ParseError(e) => write!(f, "{}", e), AssemblerError::MachineCodeError(e) => write!(f, "Machine code error: {}", e), AssemblerError::WriteError(e) => write!(f, "Write error: {}", e), + AssemblerError::ReadError(e) => write!(f, "Failed to read input file: {}", e), } } } @@ -254,7 +256,8 @@ pub struct Args { fn main() -> Result<(), AssemblerError> { let args = Args::parse(); - let program = std::fs::read_to_string(args.program).expect("Failed to read program"); + let program = + std::fs::read_to_string(args.program).map_err(|e| AssemblerError::ReadError(e))?; let assembler_result = assemble(&program); match assembler_result { Err(error) => Err(error), From d4b9f499278d3cee525010a6a4fc5bea97427b68 Mon Sep 17 00:00:00 2001 From: RandomSearch <101704343+RandomSearch18@users.noreply.github.com> Date: Tue, 18 Nov 2025 11:25:50 +0000 Subject: [PATCH 5/5] Improve WriteError error message --- src/rmc_assemble.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rmc_assemble.rs b/src/rmc_assemble.rs index 1065003..314c50d 100644 --- a/src/rmc_assemble.rs +++ b/src/rmc_assemble.rs @@ -218,7 +218,7 @@ impl fmt::Debug for AssemblerError { match self { AssemblerError::ParseError(e) => write!(f, "{}", e), AssemblerError::MachineCodeError(e) => write!(f, "Machine code error: {}", e), - AssemblerError::WriteError(e) => write!(f, "Write error: {}", e), + AssemblerError::WriteError(e) => write!(f, "Failed to write to output file: {}", e), AssemblerError::ReadError(e) => write!(f, "Failed to read input file: {}", e), } }