From e51d5866dda820fa0db2d634288fbc31542a90b1 Mon Sep 17 00:00:00 2001 From: Neta London <67196883+netalondon@users.noreply.github.com> Date: Mon, 16 Sep 2024 16:47:51 +0300 Subject: [PATCH] Fix issues with overriding OS classes (#449) * Fix issues with overriding OS classes --- simulator/src/jack/compiler.ts | 29 ++++-- simulator/src/vm/builtins.ts | 156 +++++++++++++++++++++++---------- simulator/src/vm/vm.ts | 11 ++- 3 files changed, 140 insertions(+), 56 deletions(-) diff --git a/simulator/src/jack/compiler.ts b/simulator/src/jack/compiler.ts index 03b944fad..a364b6830 100644 --- a/simulator/src/jack/compiler.ts +++ b/simulator/src/jack/compiler.ts @@ -30,7 +30,11 @@ import { isPrimitive, } from "../languages/jack.js"; import { Segment } from "../languages/vm.js"; -import { VM_BUILTINS } from "../vm/builtins.js"; +import { + VM_BUILTINS, + makeInterface, + overridesOsCorrectly, +} from "../vm/builtins.js"; import { validateSubroutine } from "./controlFlow.js"; const osClasses = new Set([ @@ -291,11 +295,25 @@ export class Compiler { } } - compileSubroutineDec(subroutine: Subroutine) { + validateSubroutineDec(subroutine: Subroutine) { this.validateReturnType( subroutine.returnType.value, subroutine.returnType.span, ); + + if (isOsClass(this.className)) { + const builtin = VM_BUILTINS[`${this.className}.${subroutine.name.value}`]; + + if (builtin && !overridesOsCorrectly(this.className, subroutine)) { + throw createError( + `OS subroutine ${this.className}.${subroutine.name.value} must follow the interface ${makeInterface(subroutine.name.value, builtin)})`, + ); + } + } + } + + compileSubroutineDec(subroutine: Subroutine) { + this.validateSubroutineDec(subroutine); switch (subroutine.type) { case "method": this.compileMethod(subroutine); @@ -426,15 +444,10 @@ export class Compiler { } this.validateArgNum( `${className}.${subroutineName}`, - isMethod ? builtin.nArgs - 1 : builtin.nArgs, + builtin.args.length, call, ); return; - } else if (isOsClass(className)) { - throw createError( - `Class ${className} doesn't contain a subroutine ${subroutineName}`, - call.span, - ); } else if (this.classes[className]) { for (const subroutine of this.classes[className].subroutines) { if (subroutine.name.value == subroutineName) { diff --git a/simulator/src/vm/builtins.ts b/simulator/src/vm/builtins.ts index c37dd30fc..60d738827 100644 --- a/simulator/src/vm/builtins.ts +++ b/simulator/src/vm/builtins.ts @@ -1,4 +1,9 @@ -import { SubroutineType } from "../languages/jack.js"; +import { + ReturnType, + Subroutine, + SubroutineType, + Type, +} from "../languages/jack.js"; import { VmMemory } from "./memory.js"; import { ERRNO } from "./os/errors.js"; import { OS } from "./os/os.js"; @@ -8,8 +13,9 @@ export type VmBuiltinFunction = (memory: VmMemory, os: OS) => number; export interface VmBuiltin { func: VmBuiltinFunction; - nArgs: number; type: SubroutineType; + args: Type[]; + returnType: ReturnType; } function getArgs(memory: VmMemory, n: number) { @@ -20,13 +26,31 @@ function getArgs(memory: VmMemory, n: number) { return args; } +export function overridesOsCorrectly(cls: string, subroutine: Subroutine) { + const builtin = VM_BUILTINS[`${cls}.${subroutine.name.value}`]; + + return ( + builtin && + builtin.args.length == subroutine.parameters.length && + builtin.args.every( + (arg, index) => arg == subroutine.parameters[index].type.value, + ) && + builtin.returnType == subroutine.returnType.value + ); +} + +export function makeInterface(name: string, builtin: VmBuiltin) { + return `${builtin.returnType} ${name}(${builtin.args.join(",")}`; +} + export const VM_BUILTINS: Record = { "Math.multiply": { func: (memory, _) => { const [a, b] = getArgs(memory, 2); return (a * b) & 0xffff; }, - nArgs: 2, + args: ["int", "int"], + returnType: "int", type: "function", }, "Math.divide": { @@ -38,7 +62,8 @@ export const VM_BUILTINS: Record = { } return Math.floor(a / b) & 0xffff; }, - nArgs: 2, + args: ["int", "int"], + returnType: "int", type: "function", }, "Math.min": { @@ -46,7 +71,8 @@ export const VM_BUILTINS: Record = { const [a, b] = getArgs(memory, 2); return Math.min(a, b) & 0xffff; }, - nArgs: 2, + args: ["int", "int"], + returnType: "int", type: "function", }, "Math.max": { @@ -54,7 +80,8 @@ export const VM_BUILTINS: Record = { const [a, b] = getArgs(memory, 2); return Math.max(a, b) & 0xffff; }, - nArgs: 2, + args: ["int", "int"], + returnType: "int", type: "function", }, "Math.sqrt": { @@ -66,7 +93,8 @@ export const VM_BUILTINS: Record = { } return Math.floor(Math.sqrt(x)) & 0xffff; }, - nArgs: 1, + args: ["int"], + returnType: "int", type: "function", }, "Math.abs": { @@ -74,7 +102,8 @@ export const VM_BUILTINS: Record = { const [x] = getArgs(memory, 1); return Math.abs(x) & 0xffff; }, - nArgs: 1, + args: ["int"], + returnType: "int", type: "function", }, "Screen.clearScreen": { @@ -82,7 +111,8 @@ export const VM_BUILTINS: Record = { os.screen.clear(); return 0; }, - nArgs: 0, + args: [], + returnType: "void", type: "function", }, "Screen.setColor": { @@ -91,7 +121,8 @@ export const VM_BUILTINS: Record = { os.screen.color = color !== 0; return 0; }, - nArgs: 1, + args: ["boolean"], + returnType: "void", type: "function", }, "Screen.drawPixel": { @@ -100,7 +131,8 @@ export const VM_BUILTINS: Record = { os.screen.drawPixel(x, y); return 0; }, - nArgs: 2, + args: ["int", "int"], + returnType: "void", type: "function", }, "Screen.drawLine": { @@ -109,7 +141,8 @@ export const VM_BUILTINS: Record = { os.screen.drawLine(x1, y1, x2, y2); return 0; }, - nArgs: 4, + args: ["int", "int", "int", "int"], + returnType: "void", type: "function", }, "Screen.drawRectangle": { @@ -118,7 +151,8 @@ export const VM_BUILTINS: Record = { os.screen.drawRect(x1, y1, x2, y2); return 0; }, - nArgs: 4, + args: ["int", "int", "int", "int"], + returnType: "void", type: "function", }, "Screen.drawCircle": { @@ -127,7 +161,8 @@ export const VM_BUILTINS: Record = { os.screen.drawCircle(x, y, r); return 0; }, - nArgs: 3, + args: ["int", "int", "int"], + returnType: "void", type: "function", }, "Memory.peek": { @@ -135,7 +170,8 @@ export const VM_BUILTINS: Record = { const [address] = getArgs(memory, 1); return memory.get(address); }, - nArgs: 1, + args: ["int"], + returnType: "int", type: "function", }, "Memory.poke": { @@ -144,7 +180,8 @@ export const VM_BUILTINS: Record = { memory.set(address, value); return 0; }, - nArgs: 2, + args: ["int", "int"], + returnType: "void", type: "function", }, "Memory.alloc": { @@ -152,7 +189,8 @@ export const VM_BUILTINS: Record = { const [size] = getArgs(memory, 1); return os.memory.alloc(size); }, - nArgs: 1, + args: ["int"], + returnType: "Array", type: "function", }, "Memory.deAlloc": { @@ -161,7 +199,8 @@ export const VM_BUILTINS: Record = { os.memory.deAlloc(address); return 0; }, - nArgs: 1, + args: ["Array"], + returnType: "void", type: "function", }, "Array.new": { @@ -173,7 +212,8 @@ export const VM_BUILTINS: Record = { } return os.memory.alloc(size); }, - nArgs: 1, + args: ["int"], + returnType: "Array", type: "constructor", }, "Array.dispose": { @@ -182,7 +222,8 @@ export const VM_BUILTINS: Record = { os.memory.deAlloc(pointer); return 0; }, - nArgs: 1, + args: [], + returnType: "void", type: "method", }, "String.new": { @@ -190,7 +231,8 @@ export const VM_BUILTINS: Record = { const [length] = getArgs(memory, 1); return os.string.new(length); }, - nArgs: 1, + args: ["int"], + returnType: "String", type: "constructor", }, "String.dispose": { @@ -199,7 +241,8 @@ export const VM_BUILTINS: Record = { os.string.dispose(pointer); return 0; }, - nArgs: 1, + args: [], + returnType: "void", type: "method", }, "String.length": { @@ -207,7 +250,8 @@ export const VM_BUILTINS: Record = { const [pointer] = getArgs(memory, 1); return os.string.length(pointer); }, - nArgs: 1, + args: [], + returnType: "int", type: "method", }, "String.charAt": { @@ -215,7 +259,8 @@ export const VM_BUILTINS: Record = { const [pointer, index] = getArgs(memory, 2); return os.string.charAt(pointer, index); }, - nArgs: 2, + args: ["int"], + returnType: "char", type: "method", }, "String.setCharAt": { @@ -224,7 +269,8 @@ export const VM_BUILTINS: Record = { os.string.setCharAt(pointer, index, value); return 0; }, - nArgs: 3, + args: ["int", "char"], + returnType: "void", type: "method", }, "String.appendChar": { @@ -232,7 +278,8 @@ export const VM_BUILTINS: Record = { const [pointer, value] = getArgs(memory, 2); return os.string.appendChar(pointer, value); }, - nArgs: 2, + args: ["char"], + returnType: "String", type: "method", }, "String.eraseLastChar": { @@ -241,7 +288,8 @@ export const VM_BUILTINS: Record = { os.string.eraseLastChar(pointer); return 0; }, - nArgs: 1, + args: [], + returnType: "void", type: "method", }, "String.intValue": { @@ -249,7 +297,8 @@ export const VM_BUILTINS: Record = { const [pointer] = getArgs(memory, 1); return os.string.intValue(pointer); }, - nArgs: 1, + args: [], + returnType: "int", type: "method", }, "String.setInt": { @@ -258,28 +307,32 @@ export const VM_BUILTINS: Record = { os.string.setInt(pointer, value); return 0; }, - nArgs: 2, + args: ["int"], + returnType: "void", type: "method", }, "String.backSpace": { func: (_, __) => { return BACKSPACE; }, - nArgs: 0, + args: [], + returnType: "char", type: "function", }, "String.doubleQuote": { func: (_, __) => { return DOUBLE_QUOTES; }, - nArgs: 0, + args: [], + returnType: "char", type: "function", }, "String.newLine": { func: (_, __) => { return NEW_LINE; }, - nArgs: 0, + args: [], + returnType: "char", type: "function", }, "Output.moveCursor": { @@ -288,7 +341,8 @@ export const VM_BUILTINS: Record = { os.output.moveCursor(i, j); return 0; }, - nArgs: 2, + args: ["int", "int"], + returnType: "void", type: "function", }, "Output.printChar": { @@ -297,7 +351,8 @@ export const VM_BUILTINS: Record = { os.output.printChar(code); return 0; }, - nArgs: 1, + args: ["char"], + returnType: "void", type: "function", }, "Output.printString": { @@ -306,7 +361,8 @@ export const VM_BUILTINS: Record = { os.output.printString(pointer); return 0; }, - nArgs: 1, + args: ["String"], + returnType: "void", type: "function", }, "Output.printInt": { @@ -315,7 +371,8 @@ export const VM_BUILTINS: Record = { os.output.printInt(value); return 0; }, - nArgs: 1, + args: ["int"], + returnType: "void", type: "function", }, "Output.println": { @@ -323,7 +380,8 @@ export const VM_BUILTINS: Record = { os.output.println(); return 0; }, - nArgs: 0, + args: [], + returnType: "void", type: "function", }, "Output.backSpace": { @@ -331,14 +389,16 @@ export const VM_BUILTINS: Record = { os.output.backspace(); return 0; }, - nArgs: 0, + args: [], + returnType: "void", type: "function", }, "Keyboard.keyPressed": { func: (_, os) => { return os.keyboard.keyPressed(); }, - nArgs: 0, + args: [], + returnType: "char", type: "function", }, "Keyboard.readChar": { @@ -346,7 +406,8 @@ export const VM_BUILTINS: Record = { os.keyboard.readChar(); return 0; }, - nArgs: 0, + args: [], + returnType: "char", type: "function", }, "Keyboard.readLine": { @@ -355,7 +416,8 @@ export const VM_BUILTINS: Record = { os.keyboard.readLine(message); return 0; }, - nArgs: 1, + args: ["String"], + returnType: "String", type: "function", }, "Keyboard.readInt": { @@ -364,7 +426,8 @@ export const VM_BUILTINS: Record = { os.keyboard.readInt(message); return 0; }, - nArgs: 1, + args: ["String"], + returnType: "int", type: "function", }, "Sys.halt": { @@ -372,7 +435,8 @@ export const VM_BUILTINS: Record = { os.sys.halt(); return 0; }, - nArgs: 0, + args: [], + returnType: "void", type: "function", }, "Sys.error": { @@ -381,7 +445,8 @@ export const VM_BUILTINS: Record = { os.sys.error(code); return 0; }, - nArgs: 1, + args: ["int"], + returnType: "void", type: "function", }, "Sys.wait": { @@ -390,7 +455,8 @@ export const VM_BUILTINS: Record = { os.sys.wait(ms); return 0; }, - nArgs: 1, + args: ["int"], + returnType: "void", type: "function", }, }; diff --git a/simulator/src/vm/vm.ts b/simulator/src/vm/vm.ts index f12ce25c9..f3c8a678f 100644 --- a/simulator/src/vm/vm.ts +++ b/simulator/src/vm/vm.ts @@ -228,12 +228,17 @@ export class Vm { for (const call of calls) { if (!functions.has(call.name)) { - if (VM_BUILTINS[call.name]) { - if (VM_BUILTINS[call.name].nArgs != call.nArgs) { + const builtin = VM_BUILTINS[call.name]; + if (builtin) { + const expectedNArgs = + builtin.type == "method" + ? builtin.args.length + 1 + : builtin.args.length; + if (expectedNArgs != call.nArgs) { return Err( createError( `OS function ${call.name} expects ${ - VM_BUILTINS[call.name].nArgs + expectedNArgs } arguments, not ${call.nArgs}`, call.span, ),