Improve asm errors

components/src/chips/memory.tsx

@@ -197,8 +197,13 @@ export const Memory = ({
       : file.name.endsWith("asm")
       ? loadAsm
       : loadBlob;
-    const bytes = await loader(source);
-    memory.loadBytes(bytes);
+    try {
+      const bytes = await loader(source);
+      memory.loadBytes(bytes);
+    } catch (e) {
+      setStatus(`Error loading memory: ${(e as Error).message}`);
+      return;
+    }
     event.target.value = ""; // Clear the input out
     setFormat(
       file.name.endsWith("hack")

components/src/stores/asm.store.ts

@@ -10,7 +10,10 @@ import {
   isAValueInstruction,
   translateInstruction,
 } from "@nand2tetris/simulator/languages/asm.js";
-import { Span } from "@nand2tetris/simulator/languages/base.js";
+import {
+  CompilationError,
+  Span,
+} from "@nand2tetris/simulator/languages/base.js";
 import { bin } from "@nand2tetris/simulator/util/twos.js";
 import { Dispatch, MutableRefObject, useContext, useMemo, useRef } from "react";
 import { useImmerReducer } from "../react.js";
@@ -164,6 +167,7 @@ export interface AsmPageState {
   compare: string;
   compareName: string | undefined;
   lineNumbers: number[];
+  error?: CompilationError;
 }
 
 export type AsmStoreDispatch = Dispatch<{
@@ -203,6 +207,13 @@ export function makeAsmStore(
       setStatus("Loaded compare file");
     },
 
+    setError(state: AsmPageState, error?: CompilationError) {
+      if (error) {
+        setStatus(error.message);
+      }
+      state.error = error;
+    },
+
     update(state: AsmPageState) {
       state.translating = translating;
       state.current = translator.current;
@@ -256,14 +267,18 @@ export function makeAsmStore(
       this.reset();
       const parseResult = ASM.parse(asm);
       if (isErr(parseResult)) {
-        setStatus(`Error parsing asm file - ${Err(parseResult).message}`);
+        dispatch.current({
+          action: "setError",
+          payload: Err(parseResult),
+        });
         compiled = false;
         return;
       }
 
       translator.load(Ok(parseResult), asm.split("\n").length);
       compiled = translator.asm.instructions.length > 0;
       setStatus("");
+      dispatch.current({ action: "setError" });
       dispatch.current({ action: "update" });
     },
 

components/src/stores/chip.store.ts

@@ -9,10 +9,7 @@ import {
   CHIP_PROJECTS,
   ChipProjects,
 } from "@nand2tetris/projects/index.js";
-import {
-  CompilationError,
-  parse as parseChip,
-} from "@nand2tetris/simulator/chip/builder.js";
+import { parse as parseChip } from "@nand2tetris/simulator/chip/builder.js";
 import {
   getBuiltinChip,
   REGISTRY,
@@ -24,7 +21,10 @@ import {
   Chip as SimChip,
 } from "@nand2tetris/simulator/chip/chip.js";
 import { Clock } from "@nand2tetris/simulator/chip/clock.js";
-import { Span } from "@nand2tetris/simulator/languages/base.js";
+import {
+  CompilationError,
+  Span,
+} from "@nand2tetris/simulator/languages/base.js";
 import { TST } from "@nand2tetris/simulator/languages/tst.js";
 import { ChipTest } from "@nand2tetris/simulator/test/chiptst.js";
 
@@ -335,11 +335,7 @@ export function makeChipStore(
       const maybeChip = await parseChip(hdl, chipName);
       if (isErr(maybeChip)) {
         const error = Err(maybeChip);
-        setStatus(
-          `${error.span?.line != undefined ? `Line ${error.span.line}: ` : ""}${
-            Err(maybeChip).message
-          }`
-        );
+        setStatus(Err(maybeChip).message);
         invalid = true;
         dispatch.current({
           action: "updateChip",

simulator/src/chip/builder.ts

@@ -6,13 +6,11 @@ import {
   Ok,
   Result,
 } from "@davidsouther/jiffies/lib/esm/result.js";
-import { ParseError, Span } from "../languages/base.js";
+import { CompilationError, createError, Span } from "../languages/base.js";
 import { HDL, HdlParse, Part, PinParts } from "../languages/hdl.js";
 import { getBuiltinChip, hasBuiltinChip } from "./builtins/index.js";
 import { Chip, Connection } from "./chip.js";
 
-const UNKNOWN_HDL_ERROR = `HDL statement has a syntax error`;
-
 function pinWidth(start: number, end: number | undefined): number | undefined {
   if (end === undefined) {
     return undefined;
@@ -26,29 +24,13 @@ function pinWidth(start: number, end: number | undefined): number | undefined {
   throw new Error(`Bus specification has start > end (${start} > ${end})`);
 }
 
-export interface CompilationError {
-  message: string;
-  span?: Span;
-}
-
-function parseErrorToCompilationError(error: ParseError) {
-  if (!error.message) {
-    return { message: UNKNOWN_HDL_ERROR, span: error.span };
-  }
-  const match = error.message.match(/Line \d+, col \d+: (?<message>.*)/);
-  if (match?.groups?.message !== undefined) {
-    return { message: match.groups.message, span: error.span };
-  }
-  return { message: error.message, span: error.span };
-}
-
 export async function parse(
   code: string,
   name?: string
 ): Promise<Result<Chip, CompilationError>> {
   const parsed = HDL.parse(code.toString());
   if (isErr(parsed)) {
-    return Err(parseErrorToCompilationError(Err(parsed)));
+    return parsed;
   }
   return build(Ok(parsed), undefined, name);
 }
@@ -174,12 +156,14 @@ function checkMultipleAssignments(
     }
   }
   if (errorIndex != undefined) {
-    return Err({
-      message: `Cannot write to pin ${pin.pin}${
-        errorIndex != -1 ? `[${errorIndex}]` : ""
-      } multiple times`,
-      span: pin.span,
-    });
+    return Err(
+      createError(
+        `Cannot write to pin ${pin.pin}${
+          errorIndex != -1 ? `[${errorIndex}]` : ""
+        } multiple times`,
+        pin.span
+      )
+    );
   }
   return Ok();
 }
@@ -210,10 +194,7 @@ class ChipBuilder {
 
   async build() {
     if (this.expectedName && this.parts.name.value != this.expectedName) {
-      return Err({
-        message: `Wrong chip name`,
-        span: this.parts.name.span,
-      });
+      return Err(createError(`Wrong chip name`, this.parts.name.span));
     }
 
     if (this.parts.parts === "BUILTIN") {
@@ -234,17 +215,16 @@ class ChipBuilder {
     for (const part of this.parts.parts) {
       const builtin = await loadChip(part.name, this.fs);
       if (isErr(builtin)) {
-        return Err({
-          message: `Undefined chip name: ${part.name}`,
-          span: part.span,
-        });
+        return Err(createError(`Undefined chip name: ${part.name}`, part.span));
       }
       const partChip = Ok(builtin);
       if (partChip.name == this.chip.name) {
-        return Err({
-          message: `Cannot use chip ${partChip.name} to implement itself`,
-          span: part.span,
-        });
+        return Err(
+          createError(
+            `Cannot use chip ${partChip.name} to implement itself`,
+            part.span
+          )
+        );
       }
       const result = this.wirePart(part, partChip);
       if (isErr(result)) {
@@ -298,10 +278,9 @@ class ChipBuilder {
         return result;
       }
     } else {
-      return Err({
-        message: `Undefined input/output pin name: ${lhs.pin}`,
-        span: lhs.span,
-      });
+      return Err(
+        createError(`Undefined input/output pin name: ${lhs.pin}`, lhs.span)
+      );
     }
     if (!isConstant(rhs.pin)) {
       this.wires.push({ chip: partChip, lhs, rhs });
@@ -364,10 +343,12 @@ class ChipBuilder {
     } else {
       // rhs is necessarily an internal pin
       if (rhs.start !== undefined || rhs.end !== undefined) {
-        return Err({
-          message: `Cannot write to sub bus of internal pin ${rhs.pin}`,
-          span: rhs.span,
-        });
+        return Err(
+          createError(
+            `Cannot write to sub bus of internal pin ${rhs.pin}`,
+            rhs.span
+          )
+        );
       }
       // track internal pin creation to detect undefined pins
       const pinData = this.internalPins.get(rhs.pin);
@@ -380,10 +361,9 @@ class ChipBuilder {
         });
       } else {
         if (pinData.isDefined) {
-          return Err({
-            message: `Internal pin ${rhs.pin} already defined`,
-            span: rhs.span,
-          });
+          return Err(
+            createError(`Internal pin ${rhs.pin} already defined`, rhs.span)
+          );
         }
         pinData.isDefined = true;
         pinData.width = width;
@@ -394,47 +374,43 @@ class ChipBuilder {
 
   private validateWriteTarget(rhs: PinParts): Result<void, CompilationError> {
     if (this.chip.isInPin(rhs.pin)) {
-      return Err({
-        message: `Cannot write to input pin ${rhs.pin}`,
-        span: rhs.span,
-      });
+      return Err(createError(`Cannot write to input pin ${rhs.pin}`, rhs.span));
     }
     if (isConstant(rhs.pin)) {
-      return Err({
-        message: `Illegal internal pin name: ${rhs.pin}`,
-        span: rhs.span,
-      });
+      return Err(
+        createError(`Illegal internal pin name: ${rhs.pin}`, rhs.span)
+      );
     }
     return Ok();
   }
 
   private validateInputSource(rhs: PinParts): Result<void, CompilationError> {
     if (this.chip.isOutPin(rhs.pin)) {
-      return Err({
-        message: `Cannot use output pin as input`,
-        span: rhs.span,
-      });
+      return Err(createError(`Cannot use output pin as input`, rhs.span));
     } else if (!this.chip.isInPin(rhs.pin) && rhs.start != undefined) {
-      return Err({
-        message: isConstant(rhs.pin)
-          ? `Cannot use sub bus of constant bus`
-          : `Cannot use sub bus of internal pin ${rhs.pin} as input`,
-        span: rhs.span,
-      });
+      return Err(
+        createError(
+          isConstant(rhs.pin)
+            ? `Cannot use sub bus of constant bus`
+            : `Cannot use sub bus of internal pin ${rhs.pin} as input`,
+          rhs.span
+        )
+      );
     }
     return Ok();
   }
 
   private validateInternalPins(): Result<void, CompilationError> {
     for (const [name, pinData] of this.internalPins) {
       if (!pinData.isDefined) {
-        return Err({
-          message:
+        return Err(
+          createError(
             name.toLowerCase() == "true" || name.toLowerCase() == "false"
               ? `The constants ${name.toLowerCase()} must be in lower-case`
               : `Undefined internal pin name: ${name}`,
-          span: pinData.firstUse,
-        });
+            pinData.firstUse
+          )
+        );
       }
     }
     return Ok();
@@ -449,12 +425,14 @@ class ChipBuilder {
         this.chip.get(wire.rhs.pin)?.width ??
         this.internalPins.get(wire.rhs.pin)?.width;
       if (lhsWidth != rhsWidth) {
-        return Err({
-          message: `Different bus widths: ${display(
-            wire.lhs
-          )}(${lhsWidth}) and ${display(wire.rhs)}(${rhsWidth})`,
-          span: wire.lhs.span,
-        });
+        return Err(
+          createError(
+            `Different bus widths: ${display(
+              wire.lhs
+            )}(${lhsWidth}) and ${display(wire.rhs)}(${rhsWidth})`,
+            wire.lhs.span
+          )
+        );
       }
     }
     return Ok();

simulator/src/languages/base.ts

@@ -54,16 +54,32 @@ baseSemantics.addAttribute("String", {
   },
 });
 
-export interface ParseError {
-  message: string | undefined;
+export interface CompilationError {
+  message: string;
   span?: Span;
 }
 
+const UNKNOWN_HDL_ERROR = `HDL statement has a syntax error`;
+
+export function createError(
+  description: string,
+  span?: Span
+): CompilationError {
+  const match = description.match(/Line \d+, col \d+: (?<message>.*)/);
+  const message = match?.groups?.message ? match.groups.message : description;
+  return {
+    message: `${
+      span?.line != undefined ? `Line ${span.line}: ` : ""
+    }${message}`,
+    span: span,
+  };
+}
+
 export function makeParser<ResultType>(
   grammar: ohm.Grammar,
   semantics: ohm.Semantics,
   property: (obj: ohm.Dict) => ResultType = ({ root }) => root
-): (source: string) => Result<ResultType, ParseError> {
+): (source: string) => Result<ResultType, CompilationError> {
   return function parse(source) {
     try {
       const match = grammar.match(source);
@@ -72,10 +88,12 @@ export function makeParser<ResultType>(
         const parse = property(parsed);
         return Ok(parse);
       } else {
-        return Err({
-          message: match.shortMessage,
-          span: span(match.getInterval()),
-        });
+        return Err(
+          createError(
+            match.shortMessage ?? UNKNOWN_HDL_ERROR,
+            span(match.getInterval())
+          )
+        );
       }
     } catch (e) {
       return Err(e as Error);