From 8b0267d8ee281d4963fafd0e5178ab5e02b66d95 Mon Sep 17 00:00:00 2001
From: Cassie Jones <code@witchoflight.com>
Date: Thu, 16 Jan 2020 23:47:45 -0500
Subject: [PATCH] Improve assembler/disassembler errors

---
 toolchain/src/inst/decode.rs | 14 +++++++++++---
 toolchain/src/js.rs          | 31 ++++++++++++++++++++++++-------
 toolchain/src/lib.rs         | 18 ++++++++++++++++--
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/toolchain/src/inst/decode.rs b/toolchain/src/inst/decode.rs
index 46bbc55..61b1ded 100644
--- a/toolchain/src/inst/decode.rs
+++ b/toolchain/src/inst/decode.rs
@@ -15,11 +15,11 @@ fn special(x: u8) -> io::Result<Special> {
         0b001 => Ok(Special::Data(Data::D2)),
         0b010 => Ok(Special::Code),
         0b011 => Err(io::Error::new(
-            io::ErrorKind::Other,
+            io::ErrorKind::InvalidInput,
             "Reserved special register",
         )),
         _ => Err(io::Error::new(
-            io::ErrorKind::Other,
+            io::ErrorKind::InvalidInput,
             "Invalid special register",
         )),
     }
@@ -96,7 +96,15 @@ impl Decode for Inst {
             (o @ 0b1010_1, r) | (o @ 0b1011_1, r) => Inst::StD(D2, reg(r), flag(o)),
             (0b1111_0..=0b1111_1, _) => {
                 let mut imm_byte = [0];
-                reader.read_exact(&mut imm_byte)?;
+                reader
+                    .read_exact(&mut imm_byte)
+                    .map_err(|e| match e.kind() {
+                        io::ErrorKind::UnexpectedEof => io::Error::new(
+                            io::ErrorKind::InvalidInput,
+                            "Extended instruction missing operand",
+                        ),
+                        _ => e,
+                    })?;
                 let imm = imm_byte[0];
                 match op_byte[0] & 0xf {
                     0b0000 => Inst::AluI(AluI::And(imm)),
diff --git a/toolchain/src/js.rs b/toolchain/src/js.rs
index 228adfd..c8d8c57 100644
--- a/toolchain/src/js.rs
+++ b/toolchain/src/js.rs
@@ -1,9 +1,9 @@
 use super::{
-    inst::{ Encode, Inst},
+    inst::{Encode, Inst},
     Error,
 };
-use js_sys;
-use std::io::{Cursor, Write};
+use js_sys::{self, Reflect};
+use std::io::{self, Cursor, Write};
 use wasm_bindgen::prelude::*;
 
 #[wasm_bindgen]
@@ -14,8 +14,10 @@ pub fn assemble(code: &str) -> Result<Vec<u8>, JsValue> {
         if line.is_empty() {
             continue;
         }
-        let inst = Inst::parse(line)
-            .ok_or_else(|| js_sys::Error::new(&format!("Error parsing line {}", i + 1)))?;
+        let inst = Inst::parse(line).ok_or_else(|| Error::Io {
+            position: i,
+            error: io::Error::new(io::ErrorKind::InvalidInput, "Parse error"),
+        })?;
         inst.encode(&mut result)
             .expect("can always write into a vector");
     }
@@ -23,7 +25,7 @@ pub fn assemble(code: &str) -> Result<Vec<u8>, JsValue> {
 }
 
 #[wasm_bindgen]
-pub fn disassemble(data: Vec<u8>) -> Result<String, JsValue> {
+pub fn disassemble(data: &[u8]) -> Result<String, JsValue> {
     let mut input = Cursor::new(data);
     let mut output = Cursor::new(Vec::new());
     for inst in super::disassemble(&mut input)? {
@@ -37,8 +39,23 @@ impl From<Error> for JsValue {
     fn from(e: Error) -> JsValue {
         match e {
             Error::Io { position, error } => {
-                js_sys::Error::new(&format!("Error at byte {}: {}", position, error)).into()
+                let obj = js_sys::Error::new(&format!("{}", error)).into();
+                Reflect::set(&obj, &"position".into(), &(position as u32).into()).unwrap();
+                obj
             }
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    #[test]
+    fn test_assemble() {
+        // test this
+    }
+
+    #[test]
+    fn test_disassemble() {
+        assert!(disassemble(&[0xff]).is_err());
+    }
+}
diff --git a/toolchain/src/lib.rs b/toolchain/src/lib.rs
index 5344f0a..ee96f75 100644
--- a/toolchain/src/lib.rs
+++ b/toolchain/src/lib.rs
@@ -2,8 +2,8 @@ pub mod inst;
 #[cfg(target_arch = "wasm32")]
 pub mod js;
 
-pub use inst::Inst;
 use inst::Decode;
+pub use inst::Inst;
 use std::io::{self, Read};
 
 pub enum Error {
@@ -14,12 +14,13 @@ pub fn disassemble(reader: &mut impl io::Read) -> Result<Vec<Inst>, Error> {
     let mut results = Vec::new();
     let mut reader = PositionedRead::new(reader);
     loop {
+        let position = reader.position;
         match Inst::decode(&mut reader) {
             Ok(inst) => results.push(inst),
             Err(err) if err.kind() == io::ErrorKind::UnexpectedEof => break,
             Err(err) => {
                 return Err(Error::Io {
-                    position: reader.position,
+                    position,
                     error: err,
                 })
             }
@@ -46,3 +47,16 @@ impl<'a, R: Read> Read for PositionedRead<'a, R> {
         Ok(read)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use io::{self, Cursor};
+    #[test]
+    fn test_disassemble_errors_at_end() {
+        let mut cursor = Cursor::new(&[0xff]);
+        let err = Inst::decode(&mut cursor).unwrap_err();
+        assert_eq!(err.kind(), io::ErrorKind::InvalidInput);
+        assert_eq!(err.get_ref().map(|x| x.to_string()), Some("Extended instruction missing operand".into()));
+    }
+}
-- 
2.43.2