renames, improve module API (avoid cloning)

crsn/src/asm/instr/flatten.rs

@@ -4,15 +4,15 @@ use std::sync::atomic::AtomicU32;
 use crate::asm::data::{Rd, SrcDisp};
 use crate::asm::data::literal::{Label, Value};
 use crate::asm::error::{AsmError, Error};
-use crate::asm::instr::{Cond, HLOp, Instr, Op, Routine};
+use crate::asm::instr::{Cond, OpWrapper, Instr, Op, Routine};
 
 /// A trait for something that can turn into multiple instructions
 pub trait Flatten {
-    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<HLOp>, Error>;
+    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<OpWrapper>, Error>;
 }
 
 impl Flatten for Instr {
-    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<HLOp>, Error> {
+    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<OpWrapper>, Error> {
         let mut ops = vec![self.op];
 
         if let Some(branches) = self.branches {
@@ -29,18 +29,18 @@ impl Flatten for Instr {
                 } else {
                     Label::unique(label_num)
                 };
-                ops.push(HLOp::JumpIf(!cond, next_lbl.clone()));
+                ops.push(OpWrapper::JumpIf(!cond, next_lbl.clone()));
 
                 for branch_instr in branch {
                     ops.extend(branch_instr.flatten(label_num)?);
                 }
 
                 if cnt != branch_count - 1 {
-                    ops.push(HLOp::Jump(end_lbl.clone()));
-                    ops.push(HLOp::Label(next_lbl));
+                    ops.push(OpWrapper::Jump(end_lbl.clone()));
+                    ops.push(OpWrapper::Label(next_lbl));
                 }
             }
-            ops.push(HLOp::Label(end_lbl));
+            ops.push(OpWrapper::Label(end_lbl));
         }
 
         Ok(ops)
@@ -48,7 +48,7 @@ impl Flatten for Instr {
 }
 
 impl Flatten for Routine {
-    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<HLOp>, Error> {
+    fn flatten(self, label_num: &AtomicU32) -> Result<Vec<OpWrapper>, Error> {
         let mut ops = vec![
             Op::Routine(self.name.clone()).into(),
         ];
@@ -63,10 +63,10 @@ impl Flatten for Routine {
 }
 
 /// Convert jumps to relative skips
-pub fn lower(ops: Vec<HLOp>) -> Result<Vec<Op>, Error> {
+pub fn lower(ops: Vec<OpWrapper>) -> Result<Vec<Op>, Error> {
     let mut label_positions = HashMap::<Label, usize>::new();
     for (n, op) in ops.iter().enumerate() {
-        if let HLOp::Label(name) = op {
+        if let OpWrapper::Label(name) = op {
             label_positions.insert(name.clone(), n - label_positions.len());
         }
     }
@@ -75,10 +75,10 @@ pub fn lower(ops: Vec<HLOp>) -> Result<Vec<Op>, Error> {
     let mut skipped = 0;
     for (n, op) in ops.into_iter().enumerate() {
         match op {
-            HLOp::Label(_) => {
+            OpWrapper::Label(_) => {
                 skipped += 1;
             }
-            HLOp::Jump(target) => {
+            OpWrapper::Jump(target) => {
                 if let Some(dest) = label_positions.get(&target) {
                     let skip = *dest as isize - n as isize + skipped;
                     cleaned.push(Op::Skip(Rd::new(SrcDisp::Immediate(skip as Value))));
@@ -86,7 +86,7 @@ pub fn lower(ops: Vec<HLOp>) -> Result<Vec<Op>, Error> {
                     return Err(Error::Asm(AsmError::LabelNotDefined(target)));
                 }
             }
-            HLOp::JumpIf(cond, target) => {
+            OpWrapper::JumpIf(cond, target) => {
                 if let Some(dest) = label_positions.get(&target) {
                     let skip = *dest as isize - n as isize + skipped;
                     cleaned.push(Op::SkipIf(cond, Rd::new(SrcDisp::Immediate(skip as Value))));
@@ -94,7 +94,7 @@ pub fn lower(ops: Vec<HLOp>) -> Result<Vec<Op>, Error> {
                     return Err(Error::Asm(AsmError::LabelNotDefined(target)));
                 }
             }
-            HLOp::L(op) => {
+            OpWrapper::Op(op) => {
                 cleaned.push(op);
             }
         }

crsn/src/asm/instr/mod.rs

@@ -1,7 +1,7 @@
 pub use cond::Cond;
 pub use flatten::Flatten;
 pub use flatten::lower;
-pub use op::HLOp;
+pub use op::OpWrapper;
 pub use op::Op;
 
 use crate::asm::data::literal::RoutineName;
@@ -13,7 +13,7 @@ mod flatten;
 /// A higher-level instruction
 #[derive(Debug, Clone)]
 pub struct Instr {
-    pub op: HLOp,
+    pub op: OpWrapper,
     pub branches: Option<Vec<(Cond, Vec<Instr>)>>,
 }
 

crsn/src/asm/instr/op.rs

@@ -14,10 +14,11 @@ use crate::asm::instr::Cond;
 use crate::runtime::fault::Fault;
 use crate::runtime::frame::{CallStack, StackFrame};
 use crate::runtime::program::Program;
+use crate::asm::parse::arg_parser::ArgParser;
 
 /// A higher level simple opration
 #[derive(Clone, Debug)]
-pub enum HLOp {
+pub enum OpWrapper {
     /// Mark a jump target.
     Label(Label),
     /// Jump to a label
@@ -25,7 +26,7 @@ pub enum HLOp {
     /// Jump to a label if a flag is set
     JumpIf(Cond, Label),
     /// Low level op
-    L(Op),
+    Op(Op),
 }
 
 /// A low level instruction
@@ -65,9 +66,9 @@ pub enum Op {
 }
 
 /// Make "into" work
-impl From<Op> for HLOp {
+impl From<Op> for OpWrapper {
     fn from(op: Op) -> Self {
-        HLOp::L(op)
+        OpWrapper::Op(op)
     }
 }
 
@@ -76,7 +77,20 @@ pub trait OpTrait: Clone + Debug + Send + 'static {
     fn execute(&self, program: &Program, call_stack: &mut CallStack, frame: &mut StackFrame) -> Result<(), Fault>;
 }
 
+pub enum ParseOpResult {
+    Parsed(Box<dyn OpTrait>),
+    Unknown(ArgParser),
+}
+
 #[clonable]
-pub trait OpParser: Clone + Debug + Send + 'static {
-    fn parse_op(&self, keyword: &str, far: bool, arg_tokens: Vec<Sexp>) -> Result<Box<dyn OpTrait>, Error>;
+pub trait AsmModule: Clone + Debug + Send + 'static {
+    /// Get name of the module
+    fn name(&self) -> &'static str;
+
+    /// Parse an op.
+    /// If the keyword matches and the function decides to parse the instruction, it must consume
+    /// the argument list and either return Ok or Err.
+    ///
+    /// If the instruction keyword is not recognized, return Unknown with the unchanged argument list.
+    fn parse_op(&self, keyword: &str, arg_tokens: ArgParser) -> Result<ParseOpResult, Error>;
 }

crsn/src/asm/mod.rs

@@ -1,5 +1,5 @@
 use crate::asm::instr::{lower, Op};
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::op::AsmModule;
 
 pub mod data;
 pub mod error;
@@ -8,7 +8,7 @@ pub mod parse;
 pub mod patches;
 
 /// Parse a program from string and assemble a low level instruction sequence from it.
-pub fn assemble(source: &str, parsers: &[Box<dyn OpParser>]) -> Result<Vec<Op>, error::Error> {
+pub fn assemble(source: &str, parsers: &[Box<dyn AsmModule>]) -> Result<Vec<Op>, error::Error> {
     let parsed = parse::parse(source, parsers)?;
     Ok(lower(parsed)?)
 }
@@ -19,7 +19,7 @@ mod tests {
 
     use crate::asm::data::{DstDisp, Rd, Register, SrcDisp, Wr};
     use crate::asm::data::literal::{Addr, Label};
-    use crate::asm::instr::{Flatten, HLOp, Instr, lower, Op};
+    use crate::asm::instr::{Flatten, OpWrapper, Instr, lower, Op};
     use crate::asm::instr::Cond;
     use crate::asm::parse::{parse, parse_instructions};
 
@@ -28,7 +28,7 @@ mod tests {
         let parsed = parse("
             ()
         ").unwrap();
-        assert_eq!(Vec::<HLOp>::new(), parsed);
+        assert_eq!(Vec::<OpWrapper>::new(), parsed);
     }
 
     #[test]
@@ -39,8 +39,8 @@ mod tests {
             )
         ").unwrap();
         assert_eq!(vec![
-            HLOp::L(Op::Routine("hello".into())),
-            HLOp::L(Op::Barrier(Some("Routine \"hello\" overrun".into())))
+            OpWrapper::Op(Op::Routine("hello".into())),
+            OpWrapper::Op(Op::Barrier(Some("Routine \"hello\" overrun".into())))
         ], parsed);
 
         let parsed = parse("
@@ -50,10 +50,10 @@ mod tests {
             )
         ").unwrap();
         assert_eq!(vec![
-            HLOp::L(Op::Routine("hello".into())),
-            HLOp::L(Op::Barrier(Some("Routine \"hello\" overrun".into()))),
-            HLOp::L(Op::Routine("world".into())),
-            HLOp::L(Op::Barrier(Some("Routine \"world\" overrun".into())))
+            OpWrapper::Op(Op::Routine("hello".into())),
+            OpWrapper::Op(Op::Barrier(Some("Routine \"hello\" overrun".into()))),
+            OpWrapper::Op(Op::Routine("world".into())),
+            OpWrapper::Op(Op::Barrier(Some("Routine \"world\" overrun".into())))
         ], parsed);
     }
 
@@ -79,73 +79,73 @@ mod tests {
             )
         ").unwrap();
         assert_eq!(vec![
-            HLOp::L(Op::Routine("move".into())),
+            OpWrapper::Op(Op::Routine("move".into())),
             // (mov r0 r1)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(0))),
                 Rd::new(SrcDisp::Register(Register::Gen(1))),
             )),
             // (mov r15 7)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(15))),
                 Rd::new(SrcDisp::Immediate(7)),
             )),
             // (mov r15 0xabcd)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(15))),
                 Rd::new(SrcDisp::Immediate(0xabcd)),
             )),
             // (mov r7 0b11110000)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(7))),
                 Rd::new(SrcDisp::Immediate(0b11110000)),
             )),
             // (mov r7 arg1)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(7))),
                 Rd::new(SrcDisp::Register(Register::Arg(1))),
             )),
             // (mov r255 arg255)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(255))),
                 Rd::new(SrcDisp::Register(Register::Arg(255))),
             )),
             // (mov r7 res0)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(7))),
                 Rd::new(SrcDisp::Register(Register::Res(0))),
             )),
             // (mov r7 res255)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(7))),
                 Rd::new(SrcDisp::Register(Register::Res(255))),
             )),
             // (mov @r0 @r0)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::RegisterPtr(Register::Gen(0))),
                 Rd::new(SrcDisp::RegisterPtr(Register::Gen(0))),
             )),
             // (mov @r0 @arg0)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::RegisterPtr(Register::Gen(0))),
                 Rd::new(SrcDisp::RegisterPtr(Register::Arg(0))),
             )),
             // (mov @r0 @res0)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::RegisterPtr(Register::Gen(0))),
                 Rd::new(SrcDisp::RegisterPtr(Register::Res(0))),
             )),
             // (mov @123456 @0x123456)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::ImmediatePtr(Addr(123456))),
                 Rd::new(SrcDisp::ImmediatePtr(Addr(0x123456))),
             )),
             //  (mov @0b010101 @0b010101)
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::ImmediatePtr(Addr(0b010101))),
                 Rd::new(SrcDisp::ImmediatePtr(Addr(0b010101))),
             )),
-            HLOp::L(Op::Barrier(Some("Routine \"move\" overrun".into()))),
+            OpWrapper::Op(Op::Barrier(Some("Routine \"move\" overrun".into()))),
         ], parsed);
     }
 
@@ -153,7 +153,7 @@ mod tests {
         Ok(parse_instructions(vec![sexp::parse(src)?])?.remove(0))
     }
 
-    fn parse_single_op(src: &str) -> anyhow::Result<Vec<HLOp>> {
+    fn parse_single_op(src: &str) -> anyhow::Result<Vec<OpWrapper>> {
         let num = AtomicU32::new(0);
         Ok(parse_single_instr(src)?.flatten(&num)?)
     }
@@ -162,7 +162,7 @@ mod tests {
     fn test_parse_single() {
         let parsed = parse_single_op("(mov r0 r1)").unwrap();
         assert_eq!(vec![
-            HLOp::L(Op::Mov(
+            OpWrapper::Op(Op::Mov(
                 Wr::new(DstDisp::Register(Register::Gen(0))),
                 Rd::new(SrcDisp::Register(Register::Gen(1))),
             )),
@@ -176,7 +176,7 @@ mod tests {
         ").unwrap();
         assert_eq!(
             Instr {
-                op: HLOp::L(Op::Cmp(
+                op: OpWrapper::Op(Op::Cmp(
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(1))),
                 )),
@@ -185,14 +185,14 @@ mod tests {
                         Cond::Equal,
                         vec![
                             Instr {
-                                op: HLOp::L(Op::Mov(
+                                op: OpWrapper::Op(Op::Mov(
                                     Wr::new(DstDisp::Register(Register::Gen(0))),
                                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                                 )),
                                 branches: None,
                             },
                             Instr {
-                                op: HLOp::L(Op::Mov(
+                                op: OpWrapper::Op(Op::Mov(
                                     Wr::new(DstDisp::Register(Register::Gen(1))),
                                     Rd::new(SrcDisp::Register(Register::Gen(2))),
                                 )),
@@ -204,14 +204,14 @@ mod tests {
                         Cond::Greater,
                         vec![
                             Instr {
-                                op: HLOp::L(Op::Mov(
+                                op: OpWrapper::Op(Op::Mov(
                                     Wr::new(DstDisp::Register(Register::Gen(0))),
                                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                                 )),
                                 branches: None,
                             },
                             Instr {
-                                op: HLOp::L(Op::Mov(
+                                op: OpWrapper::Op(Op::Mov(
                                     Wr::new(DstDisp::Register(Register::Gen(1))),
                                     Rd::new(SrcDisp::Register(Register::Gen(1))),
                                 )),
@@ -237,31 +237,31 @@ mod tests {
         ").unwrap();
         assert_eq!(
             vec![
-                HLOp::L(Op::Cmp(
+                OpWrapper::Op(Op::Cmp(
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(1))),
                 )),
-                HLOp::JumpIf(Cond::NotEqual, Label::Numbered(1)),
-                HLOp::L(Op::Mov(
+                OpWrapper::JumpIf(Cond::NotEqual, Label::Numbered(1)),
+                OpWrapper::Op(Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                 )),
-                HLOp::L(Op::Mov(
+                OpWrapper::Op(Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(1))),
                     Rd::new(SrcDisp::Register(Register::Gen(2))),
                 )),
-                HLOp::Jump(Label::Numbered(0)),
-                HLOp::Label(Label::Numbered(1)),
-                HLOp::JumpIf(Cond::LowerOrEqual, Label::Numbered(0)),
-                HLOp::L(Op::Mov(
+                OpWrapper::Jump(Label::Numbered(0)),
+                OpWrapper::Label(Label::Numbered(1)),
+                OpWrapper::JumpIf(Cond::LowerOrEqual, Label::Numbered(0)),
+                OpWrapper::Op(Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                 )),
-                HLOp::L(Op::Mov(
+                OpWrapper::Op(Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(1))),
                     Rd::new(SrcDisp::Register(Register::Gen(1))),
                 )),
-                HLOp::Label(Label::Numbered(0)),
+                OpWrapper::Label(Label::Numbered(0)),
             ], parsed);
     }
 
@@ -286,15 +286,15 @@ mod tests {
         assert_eq!(
             vec![
                 Op::Routine("foo".into()).into(),
-                HLOp::Label(Label::Named("foo".to_string())),
-                HLOp::Label(Label::Named("unused".to_string())),
-                HLOp::Label(Label::Named("whatever".to_string())),
+                OpWrapper::Label(Label::Named("foo".to_string())),
+                OpWrapper::Label(Label::Named("unused".to_string())),
+                OpWrapper::Label(Label::Named("whatever".to_string())),
                 Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                 ).into(),
-                HLOp::Jump(Label::Named("foo".to_string())),
-                HLOp::Jump(Label::Named("foo".to_string())),
+                OpWrapper::Jump(Label::Named("foo".to_string())),
+                OpWrapper::Jump(Label::Named("foo".to_string())),
                 Op::Mov(
                     Wr::new(DstDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
@@ -303,8 +303,8 @@ mod tests {
                     Wr::new(DstDisp::Register(Register::Gen(0))),
                     Rd::new(SrcDisp::Register(Register::Gen(0))),
                 ).into(),
-                HLOp::Jump(Label::Named("whatever".to_string())),
-                HLOp::JumpIf(Cond::Equal, Label::Named("whatever".to_string())),
+                OpWrapper::Jump(Label::Named("whatever".to_string())),
+                OpWrapper::JumpIf(Cond::Equal, Label::Named("whatever".to_string())),
                 Op::Barrier(Some("Routine \"foo\" overrun".into())).into(),
             ], parsed);
 

crsn/src/asm/parse/arg_parser.rs

@@ -0,0 +1,65 @@
+use sexp::Sexp;
+use crate::asm::data::{Rd, Wr};
+use crate::asm::parse::parse_data::{parse_rd, parse_wr};
+use crate::asm::parse::sexp_expect::expect_string_atom;
+
+/// Utility for argument parsing
+pub struct ArgParser {
+    orig_len: usize,
+    args: Vec<Sexp>,
+}
+
+impl IntoIterator for ArgParser {
+    type Item = Sexp;
+    type IntoIter = std::vec::IntoIter<Sexp>;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.args.into_iter()
+    }
+}
+
+impl ArgParser {
+    /// Create a new argument parser
+    pub fn new(mut args: Vec<Sexp>) -> Self {
+        args.reverse();
+        Self {
+            orig_len: args.len(),
+            args
+        }
+    }
+
+    /// Get remaining items count
+    pub fn len(&self) -> usize {
+        self.args.len()
+    }
+
+    /// Get len before parsing started
+    pub fn orig_len(&self) -> usize {
+        self.orig_len
+    }
+
+    /// Check if parsing started
+    pub fn parsing_started(&self) -> bool {
+        self.args.len() != self.orig_len
+    }
+
+    /// Get the next entry
+    pub fn next(&mut self) -> Option<Sexp> {
+        self.args.pop()
+    }
+
+    /// Get the next string entry
+    pub fn next_string(&mut self) -> anyhow::Result<String> {
+        Ok(expect_string_atom(self.next())?)
+    }
+
+    /// Get the next entry as read location
+    pub fn next_rd(&mut self) -> anyhow::Result<Rd> {
+        parse_rd(self.next())
+    }
+
+    /// Get the next entry as write location
+    pub fn next_wr(&mut self) -> anyhow::Result<Wr> {
+        parse_wr(self.next())
+    }
+}

crsn/src/asm/parse/mod.rs

@@ -4,8 +4,8 @@ pub use parse_instr::parse_instructions;
 use parse_routines::parse_routines;
 
 use crate::asm::error::Error;
-use crate::asm::instr::{Flatten, HLOp, Routine};
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::{Flatten, OpWrapper, Routine};
+use crate::asm::instr::op::AsmModule;
 use crate::asm::parse::sexp_expect::expect_list;
 
 mod parse_cond;
@@ -15,7 +15,9 @@ mod parse_routines;
 pub mod sexp_expect;
 mod parse_op;
 
-pub fn parse(source: &str, parsers: &[Box<dyn OpParser>]) -> Result<Vec<HLOp>, Error> {
+pub mod arg_parser;
+
+pub fn parse(source: &str, parsers: &[Box<dyn AsmModule>]) -> Result<Vec<OpWrapper>, Error> {
     let root = sexp::parse(source)?;
 
     let subs: Vec<Routine> = parse_routines(expect_list(Some(root), true)?, parsers)?;

crsn/src/asm/parse/parse_cond.rs

@@ -2,12 +2,12 @@ use sexp::Sexp;
 
 use crate::asm::error::Error;
 use crate::asm::instr::{Cond, cond, Instr};
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::op::AsmModule;
 use crate::asm::parse::parse_instr::parse_instructions;
 use crate::asm::parse::sexp_expect::{expect_list, expect_string_atom};
 use crate::asm::patches::TryRemove;
 
-pub fn parse_cond_branch(tok: Sexp, parsers: &[Box<dyn OpParser>]) -> Result<(Cond, Vec<Instr>), Error> {
+pub fn parse_cond_branch(tok: Sexp, parsers: &[Box<dyn AsmModule>]) -> Result<(Cond, Vec<Instr>), Error> {
     let mut list = expect_list(Some(tok), false)?;
     let kw = expect_string_atom(list.try_remove(0))?;
 

crsn/src/asm/parse/parse_instr.rs

@@ -2,14 +2,15 @@ use sexp::Sexp;
 
 use crate::asm::error::Error;
 use crate::asm::instr::Instr;
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::op::AsmModule;
 use crate::asm::parse::parse_cond::parse_cond_branch;
 use crate::asm::parse::sexp_expect::{expect_list, expect_string_atom};
 use crate::asm::patches::SexpIsA;
 
 use super::parse_op::parse_op;
+use crate::asm::parse::arg_parser::ArgParser;
 
-pub fn parse_instructions(instrs: Vec<Sexp>, parsers: &[Box<dyn OpParser>]) -> Result<Vec<Instr>, Error> {
+pub fn parse_instructions(instrs: Vec<Sexp>, parsers: &[Box<dyn AsmModule>]) -> Result<Vec<Instr>, Error> {
     let mut parsed = vec![];
     for expr in instrs {
         let tokens = expect_list(Some(expr), false)?;
@@ -25,7 +26,7 @@ pub fn parse_instructions(instrs: Vec<Sexp>, parsers: &[Box<dyn OpParser>]) -> R
             false
         };
 
-        let arg_tokens = toki.clone().take_while(|e| e.is_atom()).collect::<Vec<_>>();
+        let arg_tokens = ArgParser::new(toki.clone().take_while(|e| e.is_atom()).collect());
         let branch_tokens = toki
             .skip_while(|e| e.is_atom())
             .take_while(|e| e.is_list());

crsn/src/asm/parse/parse_op.rs

@@ -2,33 +2,33 @@ use sexp::Sexp;
 
 use crate::asm::data::literal::{Label, RoutineName};
 use crate::asm::error::{AsmError, Error};
-use crate::asm::instr::{HLOp, Op};
+use crate::asm::instr::{OpWrapper, Op};
 use crate::asm::instr::cond::parse_cond;
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::op::{AsmModule, ParseOpResult};
 use crate::asm::parse::parse_data::{parse_label, parse_rd, parse_wr};
 use crate::asm::parse::sexp_expect::expect_string_atom;
+use crate::asm::parse::arg_parser::ArgParser;
 
-pub fn parse_op(keyword: &str, far: bool, arg_tokens_slice: Vec<Sexp>, parsers: &[Box<dyn OpParser>]) -> Result<HLOp, Error> {
-    let mut arg_tokens = arg_tokens_slice.clone().into_iter();
+pub fn parse_op(keyword: &str, far: bool, mut arg_tokens: ArgParser, parsers: &[Box<dyn AsmModule>]) -> Result<OpWrapper, Error> {
 
     Ok(match keyword {
         "j" => {
             let dest = parse_label(arg_tokens.next())?;
             if far {
-                HLOp::L(Op::FarJump(dest))
+                OpWrapper::Op(Op::FarJump(dest))
             } else {
-                HLOp::Jump(dest)
+                OpWrapper::Jump(dest)
             }
         }
 
         "call" => {
-            let dest = RoutineName(expect_string_atom(arg_tokens.next())?);
+            let dest = RoutineName(arg_tokens.next_string()?);
 
             let mut args = vec![];
             for t in arg_tokens {
                 args.push(parse_rd(Some(t))?);
             }
-            HLOp::L(Op::Call(dest, args))
+            OpWrapper::Op(Op::Call(dest, args))
         }
 
         "ret" => {
@@ -36,48 +36,48 @@ pub fn parse_op(keyword: &str, far: bool, arg_tokens_slice: Vec<Sexp>, parsers:
             for t in arg_tokens {
                 args.push(parse_rd(Some(t))?);
             }
-            HLOp::L(Op::Ret(args))
+            OpWrapper::Op(Op::Ret(args))
         }
 
         "routine" => {
-            let dest = RoutineName(expect_string_atom(arg_tokens.next())?);
-            HLOp::L(Op::Routine(dest))
+            let dest = RoutineName(arg_tokens.next_string()?);
+            OpWrapper::Op(Op::Routine(dest))
         }
 
         "s" => {
-            HLOp::L(Op::Skip(parse_rd(arg_tokens.next())?))
+            OpWrapper::Op(Op::Skip(arg_tokens.next_rd()?))
         }
 
         "sif" => {
-            let cond = parse_cond(&expect_string_atom(arg_tokens.next())?)?;
-            let offs = parse_rd(arg_tokens.next())?;
-            HLOp::L(Op::SkipIf(cond, offs))
+            let cond = parse_cond(&arg_tokens.next_string()?)?;
+            let offs = arg_tokens.next_rd()?;
+            OpWrapper::Op(Op::SkipIf(cond, offs))
         }
 
         "jif" => {
-            let cond = parse_cond(&expect_string_atom(arg_tokens.next())?)?;
+            let cond = parse_cond(&arg_tokens.next_string()?)?;
             let dest = parse_label(arg_tokens.next())?;
-            HLOp::JumpIf(cond, dest)
+            OpWrapper::JumpIf(cond, dest)
         }
 
         "barrier" => {
-            HLOp::L(Op::Barrier(match arg_tokens.next() {
+            OpWrapper::Op(Op::Barrier(match arg_tokens.next() {
                 None => None,
                 Some(s) => Some(expect_string_atom(Some(s))?.into()),
             }))
         }
 
         "fault" => {
-            HLOp::L(Op::Fault(match arg_tokens.next() {
+            OpWrapper::Op(Op::Fault(match arg_tokens.next() {
                 None => None,
                 Some(s) => Some(expect_string_atom(Some(s))?.into()),
             }))
         }
 
         "ld" => {
-            HLOp::L(Op::Move {
-                dst: parse_wr(arg_tokens.next())?,
-                src: parse_rd(arg_tokens.next())?,
+            OpWrapper::Op(Op::Move {
+                dst: arg_tokens.next_wr()?,
+                src: arg_tokens.next_rd()?,
             })
         }
 
@@ -85,19 +85,22 @@ pub fn parse_op(keyword: &str, far: bool, arg_tokens_slice: Vec<Sexp>, parsers:
             if let Some(label) = other.strip_prefix(':') {
                 let label = Label::Named(label.to_string());
                 if far {
-                    HLOp::L(Op::FarLabel(label))
+                    OpWrapper::Op(Op::FarLabel(label))
                 } else {
-                    HLOp::Label(label)
+                    OpWrapper::Label(label)
                 }
             } else {
                 for p in parsers {
-                    match p.parse_op(keyword, far, arg_tokens_slice.clone()) {
-                        Ok(op) => return Ok(HLOp::L(Op::Extension(op))),
-                        Err(Error::Asm(AsmError::UnknownInstruction)) => {
-                            /* ok, try another */
+                    arg_tokens = match p.parse_op(keyword, arg_tokens) {
+                        Ok(ParseOpResult::Parsed(op)) => return Ok(OpWrapper::Op(Op::Extension(op))),
+                        Ok(ParseOpResult::Unknown(to_reuse)) => {
+                            if to_reuse.parsing_started() {
+                                panic!("Module \"{}\" started parsing {}, but returned Unknown!", p.name(), keyword);
+                            }
+                            to_reuse
                         }
-                        Err(other) => {
-                            return Err(other);
+                        Err(err) => {
+                            return Err(err);
                         }
                     }
                 }

crsn/src/asm/parse/parse_routines.rs

@@ -2,13 +2,13 @@ use sexp::Sexp;
 
 use crate::asm::data::literal::RoutineName;
 use crate::asm::error::Error;
-use crate::asm::instr::op::OpParser;
+use crate::asm::instr::op::AsmModule;
 use crate::asm::instr::Routine;
 use crate::asm::parse::parse_instr::parse_instructions;
 use crate::asm::parse::sexp_expect::{expect_list, expect_string_atom};
 use crate::asm::patches::TryRemove;
 
-pub fn parse_routines(routines: Vec<Sexp>, parsers: &[Box<dyn OpParser>]) -> Result<Vec<Routine>, Error> {
+pub fn parse_routines(routines: Vec<Sexp>, parsers: &[Box<dyn AsmModule>]) -> Result<Vec<Routine>, Error> {
     let mut parsed = vec![];
     for rt in routines {
         let mut def = expect_list(Some(rt), false)?;

crsn_arith/src/parse.rs

@@ -1,10 +1,11 @@
 use crsn::asm::data::{Rd, Wr};
 use crsn::asm::error::{AsmError, Error};
-use crsn::asm::instr::op::{OpParser, OpTrait};
+use crsn::asm::instr::op::{AsmModule, OpTrait, ParseOpResult};
 use crsn::asm::parse::parse_data::{parse_rd, parse_wr};
 use crsn::sexp::Sexp;
 
 use crate::defs::ArithOp;
+use crsn::asm::parse::arg_parser::ArgParser;
 
 #[derive(Debug, Clone)]
 pub struct ArithOpParser {
@@ -12,31 +13,34 @@ pub struct ArithOpParser {
 }
 
 impl ArithOpParser {
-    pub fn new() -> Box<dyn OpParser> {
+    pub fn new() -> Box<dyn AsmModule> {
         Box::new(Self {
             _internal: ()
         })
     }
 }
 
-impl OpParser for ArithOpParser {
-    fn parse_op(&self, keyword: &str, _far: bool, arg_tokens_sl: Vec<Sexp>) -> Result<Box<dyn OpTrait>, Error> {
-        let mut arg_tokens = arg_tokens_sl.into_iter();
-        Ok(Box::new(match keyword {
+impl AsmModule for ArithOpParser {
+    fn name(&self) -> &'static str {
+        "arith"
+    }
+
+    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpResult, Error> {
+        Ok(ParseOpResult::Parsed(Box::new(match keyword {
             "cmp" => {
                 ArithOp::Compare {
-                    a: parse_rd(arg_tokens.next())?,
-                    b: parse_rd(arg_tokens.next())?,
+                    a: args.next_rd()?,
+                    b: args.next_rd()?,
                 }
             }
 
             "tst" => {
-                let arg = parse_rd(arg_tokens.next())?;
+                let arg = args.next_rd()?;
                 ArithOp::Test { a: arg }
             }
 
             "inc" => {
-                let dst = parse_wr(arg_tokens.next())?;
+                let dst = args.next_wr()?;
                 ArithOp::Add {
                     dst,
                     a: dst.as_rd(),
@@ -45,7 +49,7 @@ impl OpParser for ArithOpParser {
             }
 
             "dec" => {
-                let dst = parse_wr(arg_tokens.next())?;
+                let dst = args.next_wr()?;
                 ArithOp::Sub {
                     dst,
                     a: dst.as_rd(),
@@ -54,20 +58,20 @@ impl OpParser for ArithOpParser {
             }
 
             "add" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Add {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Add {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -77,20 +81,20 @@ impl OpParser for ArithOpParser {
             }
 
             "sub" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Sub {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Sub {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -100,20 +104,20 @@ impl OpParser for ArithOpParser {
             }
 
             "mul" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Mul {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Mul {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -123,11 +127,11 @@ impl OpParser for ArithOpParser {
             }
 
             "divr" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
-                        let dst = parse_wr(arg_tokens.next())?;
-                        let rem = parse_wr(arg_tokens.next())?;
-                        let div = parse_rd(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
+                        let rem = args.next_wr()?;
+                        let div = args.next_rd()?;
                         ArithOp::Div {
                             dst,
                             rem,
@@ -137,10 +141,10 @@ impl OpParser for ArithOpParser {
                     }
                     4 => {
                         ArithOp::Div {
-                            dst: parse_wr(arg_tokens.next())?,
-                            rem: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            div: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            rem: args.next_wr()?,
+                            a: args.next_rd()?,
+                            div: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -150,18 +154,18 @@ impl OpParser for ArithOpParser {
             }
 
             "div" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Div {
-                            dst: parse_wr(arg_tokens.next())?,
+                            dst: args.next_wr()?,
                             rem: Wr::discard(),
-                            a: parse_rd(arg_tokens.next())?,
-                            div: parse_rd(arg_tokens.next())?,
+                            a: args.next_rd()?,
+                            div: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
-                        let div = parse_rd(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
+                        let div = args.next_rd()?;
                         ArithOp::Div {
                             dst,
                             rem: Wr::discard(),
@@ -176,17 +180,17 @@ impl OpParser for ArithOpParser {
             }
 
             "mod" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Mod {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            div: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            div: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
-                        let div = parse_rd(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
+                        let div = args.next_rd()?;
                         ArithOp::Mod {
                             dst,
                             a: dst.as_rd(),
@@ -200,20 +204,20 @@ impl OpParser for ArithOpParser {
             }
 
             "and" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::And {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::And {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -223,20 +227,20 @@ impl OpParser for ArithOpParser {
             }
 
             "or" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Or {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Or {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -246,20 +250,20 @@ impl OpParser for ArithOpParser {
             }
 
             "xor" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Xor {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            b: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            b: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Xor {
                             dst,
                             a: dst.as_rd(),
-                            b: parse_rd(arg_tokens.next())?,
+                            b: args.next_rd()?,
                         }
                     }
                     _ => {
@@ -269,15 +273,15 @@ impl OpParser for ArithOpParser {
             }
 
             "cpl" => {
-                match arg_tokens.len() {
+                match args.len() {
                     2 => {
                         ArithOp::Cpl {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Cpl {
                             dst,
                             a: dst.as_rd(),
@@ -290,24 +294,24 @@ impl OpParser for ArithOpParser {
             }
 
             "rol" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Rol {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            n: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            n: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Rol {
                             dst,
                             a: dst.as_rd(),
-                            n: parse_rd(arg_tokens.next())?,
+                            n: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Rol {
                             dst,
                             a: dst.as_rd(),
@@ -321,24 +325,24 @@ impl OpParser for ArithOpParser {
             }
 
             "ror" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Ror {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            n: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            n: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Ror {
                             dst,
                             a: dst.as_rd(),
-                            n: parse_rd(arg_tokens.next())?,
+                            n: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Ror {
                             dst,
                             a: dst.as_rd(),
@@ -352,24 +356,24 @@ impl OpParser for ArithOpParser {
             }
 
             "lsl" | "asl" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Lsl {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            n: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            n: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Lsl {
                             dst,
                             a: dst.as_rd(),
-                            n: parse_rd(arg_tokens.next())?,
+                            n: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Lsl {
                             dst,
                             a: dst.as_rd(),
@@ -383,24 +387,24 @@ impl OpParser for ArithOpParser {
             }
 
             "lsr" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Lsr {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            n: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            n: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Lsr {
                             dst,
                             a: dst.as_rd(),
-                            n: parse_rd(arg_tokens.next())?,
+                            n: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Lsr {
                             dst,
                             a: dst.as_rd(),
@@ -414,24 +418,24 @@ impl OpParser for ArithOpParser {
             }
 
             "asr" => {
-                match arg_tokens.len() {
+                match args.len() {
                     3 => {
                         ArithOp::Asr {
-                            dst: parse_wr(arg_tokens.next())?,
-                            a: parse_rd(arg_tokens.next())?,
-                            n: parse_rd(arg_tokens.next())?,
+                            dst: args.next_wr()?,
+                            a: args.next_rd()?,
+                            n: args.next_rd()?,
                         }
                     }
                     2 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Asr {
                             dst,
                             a: dst.as_rd(),
-                            n: parse_rd(arg_tokens.next())?,
+                            n: args.next_rd()?,
                         }
                     }
                     1 => {
-                        let dst = parse_wr(arg_tokens.next())?;
+                        let dst = args.next_wr()?;
                         ArithOp::Asr {
                             dst,
                             a: dst.as_rd(),
@@ -445,8 +449,8 @@ impl OpParser for ArithOpParser {
             }
 
             _other => {
-                return Err(Error::Asm(AsmError::UnknownInstruction));
+                return Ok(ParseOpResult::Unknown(args));
             }
-        }))
+        })))
     }
 }