renaming, cleanup, fmt, move "extension module" traits and structs into a separate mod

crsn/src/asm/data/mod.rs

@@ -2,7 +2,6 @@ use std::convert::TryFrom;
 use std::fmt::{Debug, Display, Formatter};
 use std::fmt;
 
-use literal::Addr;
 pub use mask::Mask;
 pub use reg::Register;
 

crsn/src/asm/instr/op.rs

@@ -1,14 +1,8 @@
 use std::fmt::Debug;
 
-
-
-use crate::asm::error::Error;
-
-use crate::asm::parse::arg_parser::ArgParser;
 use crate::builtin::defs::BuiltinOp;
+use crate::module::{EvalRes, OpTrait};
 use crate::runtime::fault::Fault;
-
-
 use crate::runtime::run_thread::{state::RunState, ThreadInfo};
 
 /// A higher level simple opration
@@ -19,46 +13,6 @@ pub enum Op {
     Ext(Box<dyn OpTrait>),
 }
 
-pub trait OpTrait: Debug + Send + Sync + 'static {
-    fn execute(&self, ti : &ThreadInfo, state: &mut RunState) -> Result<EvalRes, Fault>;
-}
-
-pub enum ParseOpResult {
-    Parsed(Op),
-    Unknown(ArgParser),
-}
-
-pub trait AsmModule: 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>;
-}
-
-
-pub type CyclesSpent = usize;
-
-#[derive(Debug)]
-pub struct EvalRes {
-    pub cycles: CyclesSpent,
-    pub advance: i64,
-}
-
-impl Default for EvalRes {
-    fn default() -> Self {
-        Self {
-            cycles: 1,
-            advance: 1,
-        }
-    }
-}
-
-
 impl OpTrait for Op {
     fn execute(&self, ti: &ThreadInfo, state: &mut RunState) -> Result<EvalRes, Fault> {
         match self {

crsn/src/asm/mod.rs

@@ -1,7 +1,6 @@
 use std::sync::Arc;
 
-
-use crate::asm::instr::op::AsmModule;
+use crate::module::AsmModule;
 use crate::runtime::program::Program;
 
 pub mod data;

crsn/src/asm/parse/mod.rs

@@ -5,8 +5,8 @@ use parse_routines::parse_routines;
 
 use crate::asm::error::Error;
 use crate::asm::instr::{Flatten, Op, Routine};
-use crate::asm::instr::op::AsmModule;
 use crate::asm::parse::sexp_expect::expect_list;
+use crate::module::AsmModule;
 
 mod parse_cond;
 mod parse_instr;

crsn/src/asm/parse/parse_cond.rs

@@ -2,10 +2,10 @@ use sexp::Sexp;
 
 use crate::asm::error::Error;
 use crate::asm::instr::{Cond, cond, Instr};
-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;
+use crate::module::AsmModule;
 
 pub fn parse_cond_branch(tok: Sexp, parsers: &[Box<dyn AsmModule>]) -> Result<(Cond, Vec<Instr>), Error> {
     let mut list = expect_list(Some(tok), false)?;

crsn/src/asm/parse/parse_data.rs

@@ -3,7 +3,7 @@ use std::convert::TryFrom;
 use sexp::{Atom, Sexp};
 
 use crate::asm::data::{DataDisp, DstDisp, Rd, reg, SrcDisp, Wr};
-use crate::asm::data::literal::{Addr, Label};
+use crate::asm::data::literal::Label;
 use crate::asm::error::Error;
 use crate::asm::parse::sexp_expect::expect_string_atom;
 

crsn/src/asm/parse/parse_instr.rs

@@ -2,11 +2,11 @@ use sexp::Sexp;
 
 use crate::asm::error::Error;
 use crate::asm::instr::Instr;
-use crate::asm::instr::op::AsmModule;
 use crate::asm::parse::arg_parser::ArgParser;
 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 crate::module::AsmModule;
 
 use super::parse_op::parse_op;
 

crsn/src/asm/parse/parse_op.rs

@@ -1,12 +1,8 @@
-
 use crate::asm::error::Error;
 use crate::asm::instr::Op;
-
-use crate::asm::instr::op::{AsmModule, ParseOpResult};
 use crate::asm::parse::arg_parser::ArgParser;
-
-
 use crate::builtin::parse::BuiltinOpParser;
+use crate::module::{AsmModule, ParseOpRes};
 
 pub fn parse_op(keyword: &str, mut arg_tokens: ArgParser, parsers: &[Box<dyn AsmModule>]) -> Result<Op, Error> {
     // Include built-in instructions
@@ -14,8 +10,8 @@ pub fn parse_op(keyword: &str, mut arg_tokens: ArgParser, parsers: &[Box<dyn Asm
 
     for p in builtins.iter().chain(parsers) {
         arg_tokens = match p.parse_op(keyword, arg_tokens) {
-            Ok(ParseOpResult::Parsed(op)) => return Ok(op),
-            Ok(ParseOpResult::Unknown(to_reuse)) => {
+            Ok(ParseOpRes::Parsed(op)) => return Ok(op),
+            Ok(ParseOpRes::Unknown(to_reuse)) => {
                 if to_reuse.parsing_started() {
                     panic!("Module \"{}\" started parsing {}, but returned Unknown!", p.name(), keyword);
                 }

crsn/src/asm/parse/parse_routines.rs

@@ -2,11 +2,11 @@ use sexp::Sexp;
 
 use crate::asm::data::literal::RoutineName;
 use crate::asm::error::Error;
-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;
+use crate::module::AsmModule;
 
 pub fn parse_routines(routines: Vec<Sexp>, parsers: &[Box<dyn AsmModule>]) -> Result<Vec<Routine>, Error> {
     let mut parsed = vec![];

crsn/src/builtin/defs.rs

@@ -1,6 +1,6 @@
-use crate::asm::data::literal::{Label, RoutineName, DebugMsg};
-use crate::asm::instr::{Cond, Op};
 use crate::asm::data::{Rd, Wr};
+use crate::asm::data::literal::{DebugMsg, Label, RoutineName};
+use crate::asm::instr::{Cond, Op};
 
 #[derive(Debug)]
 pub enum BuiltinOp {

crsn/src/builtin/exec.rs

@@ -1,14 +1,9 @@
-
-
-
-
-use crate::asm::instr::op::{OpTrait, EvalRes};
-use crate::runtime::fault::Fault;
-use crate::runtime::frame::{StackFrame};
-
-use crate::builtin::defs::BuiltinOp;
 use crate::asm::data::literal::Addr;
-use crate::runtime::run_thread::{ThreadInfo, state::RunState};
+use crate::builtin::defs::BuiltinOp;
+use crate::module::{EvalRes, OpTrait};
+use crate::runtime::fault::Fault;
+use crate::runtime::frame::StackFrame;
+use crate::runtime::run_thread::{state::RunState, ThreadInfo};
 
 impl OpTrait for BuiltinOp {
     fn execute(&self, info: &ThreadInfo, state: &mut RunState) -> Result<EvalRes, Fault> {

crsn/src/builtin/parse.rs

@@ -1,14 +1,14 @@
+use sexp::{Atom, Sexp};
 
-use crate::asm::error::{Error};
-use crate::asm::instr::op::{AsmModule, ParseOpResult};
-use crate::asm::parse::arg_parser::ArgParser;
 use crate::asm::data::literal::{Label, RoutineName};
-use crate::asm::parse::sexp_expect::expect_string_atom;
-use crate::asm::parse::parse_data::{parse_label, parse_rd};
+use crate::asm::error::Error;
 use crate::asm::instr::cond::parse_cond;
 use crate::asm::instr::Op;
+use crate::asm::parse::arg_parser::ArgParser;
+use crate::asm::parse::parse_data::{parse_label, parse_rd};
+use crate::asm::parse::sexp_expect::expect_string_atom;
 use crate::builtin::defs::BuiltinOp;
-use sexp::{Sexp, Atom};
+use crate::module::{AsmModule, ParseOpRes};
 
 #[derive(Debug, Clone)]
 pub struct BuiltinOpParser {
@@ -28,8 +28,8 @@ impl AsmModule for BuiltinOpParser {
         "builtin"
     }
 
-    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpResult, Error> {
-        Ok(ParseOpResult::Parsed(Op::BuiltIn(match keyword {
+    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpRes, Error> {
+        Ok(ParseOpRes::Parsed(Op::BuiltIn(match keyword {
             "nop" => {
                 BuiltinOp::Nop
             }
@@ -132,10 +132,10 @@ impl AsmModule for BuiltinOpParser {
                         let label = Label::Named(label.to_string());
                         BuiltinOp::FarLabel(label)
                     } else {
-                        return Ok(ParseOpResult::Unknown(args));
+                        return Ok(ParseOpRes::Unknown(args));
                     }
                 } else {
-                    return Ok(ParseOpResult::Unknown(args));
+                    return Ok(ParseOpRes::Unknown(args));
                 }
             }
 
@@ -144,7 +144,7 @@ impl AsmModule for BuiltinOpParser {
                     let label = Label::Named(label.to_string());
                     BuiltinOp::Label(label)
                 } else {
-                    return Ok(ParseOpResult::Unknown(args));
+                    return Ok(ParseOpRes::Unknown(args));
                 }
             }
         })))

crsn/src/lib.rs

@@ -6,4 +6,5 @@ pub use sexp;
 pub mod asm;
 pub mod builtin;
 pub mod runtime;
+pub mod module;
 

crsn/src/module/eval_res.rs

@@ -0,0 +1,18 @@
+/// Cycles spent executing an instruction
+pub type CyclesSpent = usize;
+
+/// Result of execution evaluation (when Fault is not generated)
+#[derive(Debug)]
+pub struct EvalRes {
+    pub cycles: CyclesSpent,
+    pub advance: i64,
+}
+
+impl Default for EvalRes {
+    fn default() -> Self {
+        Self {
+            cycles: 1,
+            advance: 1,
+        }
+    }
+}

crsn/src/module/mod.rs

@@ -0,0 +1,36 @@
+use std::fmt::Debug;
+
+pub use eval_res::EvalRes;
+
+use crate::asm::error::Error;
+use crate::asm::instr::Op;
+use crate::asm::parse::arg_parser::ArgParser;
+use crate::runtime::fault::Fault;
+use crate::runtime::run_thread::state::RunState;
+use crate::runtime::run_thread::ThreadInfo;
+
+mod eval_res;
+
+/// Result type returned from the op parser. This is the Ok variant of a Result.
+pub enum ParseOpRes {
+    /// Parsing successful.
+    Parsed(Op),
+    /// Instruction not recognized, but there was no error.
+    Unknown(ArgParser),
+}
+
+pub trait OpTrait: Debug + Send + Sync + 'static {
+    fn execute(&self, ti: &ThreadInfo, state: &mut RunState) -> Result<EvalRes, Fault>;
+}
+
+pub trait AsmModule: 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<ParseOpRes, Error>;
+}

crsn/src/runtime/exec.rs

@@ -1,14 +1,10 @@
-
-
-use crate::asm::instr::op::{EvalRes, OpTrait};
+use crate::module::{EvalRes, OpTrait};
 use crate::runtime::fault::Fault;
-
 use crate::runtime::run_thread::RunThread;
 
 impl RunThread {
     // TODO unit tests
     pub fn eval_op(&mut self) -> Result<EvalRes, Fault> {
-
         let state = &mut self.state;
         let info = &self.info;
 

crsn/src/runtime/frame.rs

@@ -1,10 +1,7 @@
 use status::StatusFlags;
 
-use crate::asm::data::{DstDisp, Rd, Register, SrcDisp, Wr};
 use crate::asm::data::literal::{Addr, Value};
 
-use super::fault::Fault;
-
 pub(crate) mod status;
 
 pub const REG_COUNT: usize = 8;

crsn/src/runtime/frame/status.rs

@@ -90,35 +90,36 @@ impl StatusFlags {
         match cond {
             Cond::Equal => {
                 self.equal = true;
-            },
+            }
             Cond::Zero => {
                 self.zero = true;
-            },
+            }
             Cond::Lower => {
                 self.lower = true;
-            },
+            }
             Cond::Greater => {
                 self.lower = false;
                 self.equal = false;
                 self.greater = true;
-            },
+            }
             Cond::Positive => {
                 self.positive = true;
-            },
+            }
             Cond::Negative => {
                 self.negative = true;
-            },
+            }
             Cond::Overflow => {
                 self.overflow = true;
-            },
+            }
             Cond::Invalid => {
                 self.invalid = true;
-            },
+            }
             Cond::Carry => {
                 self.carry = true;
-            },
+            }
             other => {
-                panic!("cannot set cond by {:?}", other);
+                error!("Cannot set cond by {:?}", other);
+                // ...and do nothing. Don't panic, we don't want to crash the runtime!
             }
         }
     }

crsn/src/runtime/program.rs

@@ -3,8 +3,8 @@ use std::sync::Arc;
 
 use crate::asm::data::literal::{Addr, Label, RoutineName};
 use crate::asm::instr::Op;
-use crate::runtime::fault::Fault;
 use crate::builtin::defs::BuiltinOp;
+use crate::runtime::fault::Fault;
 
 #[derive(Debug)]
 pub struct Program {

crsn/src/runtime/run_thread.rs

@@ -1,15 +1,12 @@
-use std::any::{Any, TypeId};
-use std::collections::HashMap;
 use std::sync::Arc;
 use std::thread::JoinHandle;
 use std::time::Duration;
 
-use crate::asm::data::literal::{Addr, Value};
-use crate::asm::instr::op::EvalRes;
-use crate::runtime::frame::{CallStack, StackFrame, REG_COUNT, status::StatusFlags};
-use crate::runtime::program::Program;
+use crate::asm::data::literal::Addr;
+use crate::module::EvalRes;
 use crate::runtime::fault::Fault;
-use crate::asm::data::{Rd, SrcDisp, Register, Wr, DstDisp};
+use crate::runtime::frame::StackFrame;
+use crate::runtime::program::Program;
 use crate::runtime::run_thread::state::RunState;
 
 #[derive(Clone, Copy, Eq, PartialEq, Debug, Ord, PartialOrd)]

crsn/src/runtime/run_thread/state.rs

@@ -3,12 +3,9 @@ use std::collections::HashMap;
 
 use crate::asm::data::{DstDisp, Rd, Register, SrcDisp, Wr};
 use crate::asm::data::literal::{Addr, Value};
-use crate::asm::instr::op::EvalRes;
+use crate::asm::instr::Cond;
 use crate::runtime::fault::Fault;
 use crate::runtime::frame::{CallStack, REG_COUNT, StackFrame};
-use crate::runtime::program::Program;
-use crate::asm::instr::Cond;
-use crate::runtime::frame::status::StatusFlags;
 
 pub struct RunState {
     /// Active stack frame
@@ -20,6 +17,8 @@ pub struct RunState {
 }
 
 impl RunState {
+    /// Get an extension's persistent data object for mutation.
+    /// It will be created using Default if not present before.
     pub fn ext_mut<T: Send + Default + 'static>(&mut self) -> &mut T {
         if !self.ext_data.contains_key(&TypeId::of::<T>()) {
             self.ext_data.insert(TypeId::of::<T>(), Box::new(T::default()));
@@ -29,32 +28,40 @@ impl RunState {
             .downcast_mut().unwrap()
     }
 
+    /// Set a status flag. Only supports simple, positive conds (i.e. not GreaterOrEqual)
     pub fn set_flag(&mut self, cond: Cond, set: bool) {
         if set {
             self.frame.status.set(cond);
         }
     }
 
+    /// Check status flags for a condition
     pub fn test_cond(&self, cond: Cond) -> bool {
         self.frame.status.test(cond)
     }
 
+    /// Set program counter - address of the next instruction to run
     pub fn set_pc(&mut self, pc: Addr) {
         self.frame.pc = pc;
     }
 
+    /// Get program counter - address of the next instruction to run
     pub fn get_pc(&self) -> Addr {
         self.frame.pc
     }
 
+    /// Clear status flags
     pub fn clear_status(&mut self) {
         self.frame.status.clear();
     }
 
+    /// Update status flags using a variable.
+    /// The update is additive - call `clear_status()` first if desired!
     pub fn update_status(&mut self, val: Value) {
         self.frame.status.update(val);
     }
 
+    /// Read a `Rd` value
     pub fn read(&mut self, rd: Rd) -> Result<Value, Fault> {
         match rd.d() {
             SrcDisp::Immediate(v) => Ok(v),
@@ -88,6 +95,7 @@ impl RunState {
         }
     }
 
+    /// Write a value to a `Wr` location
     pub fn write(&mut self, wr: Wr, val: Value) -> Result<(), Fault> {
         trace!("WR {:?} := {}", wr, val);
 

crsn_arith/src/exec.rs

@@ -3,7 +3,7 @@ use std::ops::Rem;
 use num_traits::PrimInt;
 
 use crsn::asm::instr::Cond;
-use crsn::asm::instr::op::{EvalRes, OpTrait};
+use crsn::module::{EvalRes, OpTrait};
 use crsn::runtime::fault::Fault;
 use crsn::runtime::run_thread::{state::RunState, ThreadInfo};
 

crsn_arith/src/parse.rs

@@ -1,8 +1,8 @@
 use crsn::asm::data::{Rd, Wr};
 use crsn::asm::error::Error;
-use crsn::asm::instr::op::{AsmModule, ParseOpResult};
 use crsn::asm::instr::Op;
 use crsn::asm::parse::arg_parser::ArgParser;
+use crsn::module::{AsmModule, ParseOpRes};
 
 use crate::defs::ArithOp;
 
@@ -24,8 +24,8 @@ impl AsmModule for ArithOps {
         "arith"
     }
 
-    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpResult, Error> {
-        Ok(ParseOpResult::Parsed(Op::Ext(Box::new(match keyword {
+    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpRes, Error> {
+        Ok(ParseOpRes::Parsed(Op::Ext(Box::new(match keyword {
             "cmp" => {
                 ArithOp::Compare {
                     a: args.next_rd()?,
@@ -448,7 +448,7 @@ impl AsmModule for ArithOps {
             }
 
             _other => {
-                return Ok(ParseOpResult::Unknown(args));
+                return Ok(ParseOpRes::Unknown(args));
             }
         }))))
     }

crsn_stacks/src/exec.rs

@@ -1,12 +1,12 @@
 use std::collections::VecDeque;
 
 use crsn::asm::data::literal::Value;
-use crsn::asm::instr::op::{EvalRes, OpTrait};
+use crsn::asm::instr::Cond;
+use crsn::module::{EvalRes, OpTrait};
 use crsn::runtime::fault::Fault;
 use crsn::runtime::run_thread::{state::RunState, ThreadInfo};
 
 use crate::defs::StackOp;
-use crsn::asm::instr::Cond;
 
 struct Stacks {
     stacks: Vec<VecDeque<Value>>,

crsn_stacks/src/parse.rs

@@ -1,8 +1,7 @@
-
 use crsn::asm::error::Error;
-use crsn::asm::instr::op::{AsmModule, ParseOpResult};
 use crsn::asm::instr::Op;
 use crsn::asm::parse::arg_parser::ArgParser;
+use crsn::module::{AsmModule, ParseOpRes};
 
 use crate::defs::StackOp;
 
@@ -24,8 +23,8 @@ impl AsmModule for StackOps {
         "stacks"
     }
 
-    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpResult, Error> {
-        Ok(ParseOpResult::Parsed(Op::Ext(Box::new(match keyword {
+    fn parse_op(&self, keyword: &str, mut args: ArgParser) -> Result<ParseOpRes, Error> {
+        Ok(ParseOpRes::Parsed(Op::Ext(Box::new(match keyword {
             "push" => {
                 StackOp::Push {
                     num: args.next_rd()?,
@@ -41,7 +40,7 @@ impl AsmModule for StackOps {
             }
 
             _other => {
-                return Ok(ParseOpResult::Unknown(args));
+                return Ok(ParseOpRes::Unknown(args));
             }
         }))))
     }

launcher/src/main.rs

@@ -1,14 +1,14 @@
 #[macro_use]
 extern crate log;
 
+use std::time::Duration;
+
 use simple_logger::SimpleLogger;
 
 use crsn::asm::data::literal::Addr;
-
 use crsn::runtime::run_thread::{RunThread, ThreadToken};
 use crsn_arith::ArithOps;
 use crsn_stacks::StackOps;
-use std::time::Duration;
 
 fn main() {
     SimpleLogger::new().init().unwrap();
@@ -97,7 +97,7 @@ fn main() {
     let mut thread1 = RunThread::new(ThreadToken(0), parsed.clone(), Addr(0), &[]);
     thread1.set_speed(Duration::from_millis(250));
 
-    let thread2 = RunThread::new(ThreadToken(1), parsed.clone(), Addr(0), &[]);
+    let _thread2 = RunThread::new(ThreadToken(1), parsed.clone(), Addr(0), &[]);
 
     let a = thread1.start();
     //let b = thread2.start();