Skip to content

Commit

Permalink
Improve asm errors (#239)
Browse files Browse the repository at this point in the history 
* Add asm error highlighting
* Disable asm runbar on error
* Alert when failing to load file to memory
* Unify error message formatting
  • Loading branch information
@netalondon
netalondon authored Mar 17, 2024
1 parent 88c254b commit 0320593
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 104 deletions.
9 changes: 7 additions & 2 deletions components/src/chips/memory.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
19 changes: 17 additions & 2 deletions components/src/stores/asm.store.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -164,6 +167,7 @@ export interface AsmPageState {
compare: string;
compareName: string | undefined;
lineNumbers: number[];
error?: CompilationError;
}

export type AsmStoreDispatch = Dispatch<{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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" });
},

Expand Down
16 changes: 6 additions & 10 deletions components/src/stores/chip.store.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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";

Expand Down Expand Up @@ -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",
Expand Down
134 changes: 56 additions & 78 deletions simulator/src/chip/builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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") {
Expand All @@ -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)) {
Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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();
Expand All @@ -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();
Expand Down
32 changes: 25 additions & 7 deletions simulator/src/languages/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
Loading

0 comments on commit 0320593

Please sign in to comment.