diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 5171817f1..624737bdc 100644 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -12,7 +12,7 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: denoland/setup-deno@v1 with: @@ -37,7 +37,7 @@ jobs: - name: Install Turnt # run: pip install turnt # Use instead if pip turnt version >= 1.7 - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: cucapra/turnt path: './turnt' @@ -53,7 +53,7 @@ jobs: check: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: denoland/setup-deno@v1 with: @@ -81,7 +81,7 @@ jobs: style: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: TrueBrain/actions-flake8@master with: path: bril-txt diff --git a/.github/workflows/docs.yaml b/.github/workflows/docs.yaml index 9e31d16d8..ed8fdc0b8 100644 --- a/.github/workflows/docs.yaml +++ b/.github/workflows/docs.yaml @@ -12,7 +12,7 @@ jobs: build: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Setup mdBook uses: peaceiris/actions-mdbook@v1 with: diff --git a/.github/workflows/rust.yaml b/.github/workflows/rust.yaml index f679ddb4e..e3c395660 100644 --- a/.github/workflows/rust.yaml +++ b/.github/workflows/rust.yaml @@ -39,7 +39,7 @@ jobs: - "cd brilift && cargo build --release && make rt.o && make test TURNTARGS=-v" - "cd brilift && cargo build --release && make rt.o && make benchmark TURNTARGS=-v" steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@v1 with: toolchain: stable @@ -60,7 +60,7 @@ jobs: - name: Install Turnt # run: pip install turnt # Use instead if pip turnt version >= 1.7 - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: repository: cucapra/turnt path: './turnt' @@ -82,7 +82,7 @@ jobs: matrix: path: ["brilirs/Cargo.toml", "bril-rs/Cargo.toml", "bril-rs/bril2json/Cargo.toml", "bril-rs/brild/Cargo.toml", "brilift/Cargo.toml", "bril-rs/rs2bril/Cargo.toml", "bril-rs/brillvm/Cargo.toml"] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@v1 with: toolchain: stable diff --git a/benchmarks/core/bitshift.bril b/benchmarks/core/bitshift.bril index f7a79defa..c3c35fc82 100644 --- a/benchmarks/core/bitshift.bril +++ b/benchmarks/core/bitshift.bril @@ -7,7 +7,6 @@ .then.0: v4: int = id x; ret v4; - jmp .endif.0; .else.0: v5: int = id x; v6: int = id n; @@ -37,7 +36,6 @@ .endif.12: v22: int = id ans; ret v22; -.endif.0: } @mod(a: int, b: int): int { v0: int = id a; diff --git a/benchmarks/core/fact.bril b/benchmarks/core/fact.bril index 52f3f2d49..947725523 100644 --- a/benchmarks/core/fact.bril +++ b/benchmarks/core/fact.bril @@ -16,7 +16,6 @@ .then.0: v4: int = const 1; ret v4; - jmp .endif.0; .else.0: v5: int = id a; v6: int = id a; @@ -25,5 +24,4 @@ v9: int = call @fact v8; v10: int = mul v5 v9; ret v10; -.endif.0: } diff --git a/benchmarks/core/mod_inv.bril b/benchmarks/core/mod_inv.bril index f5ee57cf6..7f02408f3 100644 --- a/benchmarks/core/mod_inv.bril +++ b/benchmarks/core/mod_inv.bril @@ -1,4 +1,4 @@ -# ARGS: 46, 10007 +# ARGS: 46 10007 @main(n: int, p: int) { v0: int = const 2; two: int = id v0; @@ -81,10 +81,8 @@ # } # console.log(ans); # } -# +# # function mod(n: bigint, p: bigint): bigint { # return n - n / p * p; # } # - - diff --git a/bril-rs/bril2json/src/lib.rs b/bril-rs/bril2json/src/lib.rs index 032a69b67..cb1c122f9 100644 --- a/bril-rs/bril2json/src/lib.rs +++ b/bril-rs/bril2json/src/lib.rs @@ -82,17 +82,22 @@ impl Lines { self.new_lines .iter() .enumerate() + //(i+1) because line numbers start at 1 .map(|(i, j)| (i + 1, j)) .fold( ColRow { - col: index as u64, + // (index + 1) because column numbers start at 1 + col: (index + 1) as u64, + // Hard code the first row to be 1 row: 1, }, |current, (line_num, idx)| { if *idx < index { ColRow { + // (line_num + 1) because line numbers start at 1 row: (line_num + 1) as u64, - col: (index - idx) as u64, + // column values are kept relative to the previous index + col: ((index) - idx) as u64, } } else { current diff --git a/bril-ts/builder.ts b/bril-ts/builder.ts index 775948c08..63996077e 100644 --- a/bril-ts/builder.ts +++ b/bril-ts/builder.ts @@ -67,7 +67,7 @@ export class Builder { return this.buildEffect("call", args, [func], undefined); } } - + /** * Build a constant instruction. As above, the destination name is optional. */ @@ -124,6 +124,32 @@ export class Builder { this.curFunction.instrs.push(instr); } + /** + * Checks whether the last emitted instruction in the current function is the specified op code. + * Useful for checking for terminating instructions. + */ + getLastInstr(): bril.Instruction | undefined { + if (!this.curFunction) { + return undefined + } + + if (!this.curFunction.instrs) { + return undefined + } + + if (this.curFunction.instrs.length === 0) { + return undefined + } + + const last_instr : bril.Instruction | bril.Label = this.curFunction.instrs[this.curFunction.instrs.length - 1]; + + if ('label' in last_instr) { + return undefined + } + + return last_instr + } + /** * Generate an unused variable name. */ diff --git a/brili.ts b/brili.ts index 348b36f6c..ff6e2a8b3 100644 --- a/brili.ts +++ b/brili.ts @@ -764,7 +764,7 @@ function evalInstr(instr: bril.Instruction, state: State): Action { let i = getInt(instr, state.env, 0); if (i > 1114111 || i < 0 || (55295 < i && i < 57344)) { throw error(`value ${i} cannot be converted to char`); - } + } let val = String.fromCodePoint(Number(i)); state.env.set(instr.dest, val); return NEXT; @@ -873,7 +873,12 @@ function parseBool(s: string): boolean { function parseNumber(s: string): number { let f = parseFloat(s); - if (!isNaN(f)) { + // parseFloat and Number have subtly different behaviors for parsing strings + // parseFloat ignores all random garbage after any valid number + // Number accepts empty/whitespace only strings and rejects numbers with seperators + // Use both and only accept the intersection of the results? + let f2 = Number(s); + if (!isNaN(f) && f === f2) { return f; } else { throw error(`float argument to main must not be 'NaN'; got ${s}`); @@ -891,7 +896,8 @@ function parseMainArguments(expected: bril.Argument[], args: string[]) : Env { let type = expected[i].type; switch (type) { case "int": - let n: bigint = BigInt(parseInt(args[i])); + // https://dev.to/darkmavis1980/you-should-stop-using-parseint-nbf + let n: bigint = BigInt(Number(args[i])); newEnv.set(expected[i].name, n as Value); break; case "float": diff --git a/brilirs/src/check.rs b/brilirs/src/check.rs index f14a56e12..d115ea4d5 100644 --- a/brilirs/src/check.rs +++ b/brilirs/src/check.rs @@ -508,6 +508,11 @@ fn type_check_instruction<'a>( } fn type_check_func(bbfunc: &BBFunction, bbprog: &BBProgram) -> Result<(), PositionalInterpError> { + if bbfunc.name == "main" && bbfunc.return_type.is_some() { + return Err(InterpError::NonEmptyRetForFunc(bbfunc.name.clone())) + .map_err(|e| e.add_pos(bbfunc.pos.clone())); + } + let mut env: FxHashMap<&str, &Type> = FxHashMap::with_capacity_and_hasher(20, fxhash::FxBuildHasher::default()); bbfunc.args.iter().for_each(|a| { diff --git a/brilirs/src/interp.rs b/brilirs/src/interp.rs index 17c5a5596..de8d5fdab 100644 --- a/brilirs/src/interp.rs +++ b/brilirs/src/interp.rs @@ -796,11 +796,6 @@ pub fn execute_main( .map(|i| prog.get(i).unwrap()) .ok_or(InterpError::NoMainFunction)?; - if main_func.return_type.is_some() { - return Err(InterpError::NonEmptyRetForFunc(main_func.name.clone())) - .map_err(|e| e.add_pos(main_func.pos.clone())); - } - let mut env = Environment::new(main_func.num_of_vars); let heap = Heap::default(); diff --git a/test/ts/factorial.out b/test/ts/factorial.out index cf0ff0cdb..b2cd75b60 100644 --- a/test/ts/factorial.out +++ b/test/ts/factorial.out @@ -16,9 +16,7 @@ .then.0: v4: int = const 1; ret v4; - jmp .endif.0; .else.0: -.endif.0: v5: int = id x; v6: int = id x; v7: int = const 1; diff --git a/test/ts/simplified.out b/test/ts/simplified.out new file mode 100644 index 000000000..cf4144983 --- /dev/null +++ b/test/ts/simplified.out @@ -0,0 +1,13 @@ +@main { + v0: int = const 1; + call @erronious v0; + v1: int = const 0; +} +@erronious(x: int): int { + v1: bool = const true; + br v1 .then.0 .else.0; +.then.0: + v2: int = id x; + ret v2; +.else.0: +} diff --git a/test/ts/simplified.ts b/test/ts/simplified.ts new file mode 100644 index 000000000..6e53b939b --- /dev/null +++ b/test/ts/simplified.ts @@ -0,0 +1,8 @@ +erronious(1n); + +// This snippet checks that the ts compiler doesn't try to create an unreachable else branch +function erronious(x: bigint): bigint { + if (true) { + return x; + } +} diff --git a/ts2bril.ts b/ts2bril.ts index f2a1929ac..b009df1b9 100644 --- a/ts2bril.ts +++ b/ts2bril.ts @@ -145,7 +145,7 @@ function emitBril(prog: ts.Node, checker: ts.TypeChecker): bril.Program { // Check if effect statement, i.e., a call that is not a subexpression if (call.parent.kind === ts.SyntaxKind.ExpressionStatement) { - builder.buildCall(callText, + builder.buildCall(callText, values.map(v => v.dest)); return builder.buildInt(0); // Expressions must produce values } else { @@ -153,12 +153,12 @@ function emitBril(prog: ts.Node, checker: ts.TypeChecker): bril.Program { let type = brilType(decl, checker); let name = (decl.name != undefined) ? decl.name.getText() : undefined; return builder.buildCall( - callText, - values.map(v => v.dest), - type, + callText, + values.map(v => v.dest), + type, name, ); - } + } } default: throw `unsupported expression kind: ${expr.getText()}`; @@ -215,12 +215,18 @@ function emitBril(prog: ts.Node, checker: ts.TypeChecker): bril.Program { // Statement chunks. builder.buildLabel(thenLab); emit(if_.thenStatement); - builder.buildEffect("jmp", [], undefined, [endLab]); + const then_branch_terminated = builder.getLastInstr()?.op === "ret"; + if (!then_branch_terminated) { + builder.buildEffect("jmp", [], undefined, [endLab]); + } builder.buildLabel(elseLab); if (if_.elseStatement) { emit(if_.elseStatement); } - builder.buildLabel(endLab); + // The else branch otherwise just falls through without needing a target label + if (!then_branch_terminated) { + builder.buildLabel(endLab); + } break; } @@ -258,7 +264,7 @@ function emitBril(prog: ts.Node, checker: ts.TypeChecker): bril.Program { break; } - case ts.SyntaxKind.FunctionDeclaration: + case ts.SyntaxKind.FunctionDeclaration: let funcDef = node as ts.FunctionDeclaration; if (funcDef.name === undefined) { throw `no anonymous functions!`; @@ -298,7 +304,7 @@ function emitBril(prog: ts.Node, checker: ts.TypeChecker): bril.Program { case ts.SyntaxKind.ImportDeclaration: break; - + default: throw `unhandled TypeScript AST node kind ${ts.SyntaxKind[node.kind]}`; }