mlscript JavaScript code generation backlog

When working on #53, I met a lot of things that we could improved in the future. Here are them.

[ ] Find a neat way to store query results. Requirements:
- If the initialization fails, the variable should not be declared.
- Declaration should not pollute global context.
Source
[x] ~~Implement Class.Member instance calling convention. Source~~ This syntax might be changed in the new parser.
[x] Add timeout to stream.read() otherwise we will occasionally suffer from infinite loops. Source (Fixed in #126)
[x] Automatically check references to unimplemented variables. Source
[x] ~~Support merging arrays in withConstruct. Source TBD.~~ The semantics of with might be changed.
[ ] Some // TODO introduced in commits from #53. Source TBD.

As for 1 Aug, 2022. The // TODOs are as follows. They are not urgent at this moment.
- Translation of Blk. https://github.com/hkust-taco/mlscript/blob/ba99965401408470e4072d95081199c504c004a8/shared/src/main/scala/mlscript/JSBackend.scala#L177-L178
- Consider traits when sorting classes. https://github.com/hkust-taco/mlscript/blob/ba99965401408470e4072d95081199c504c004a8/shared/src/main/scala/mlscript/JSBackend.scala#L482-L485
- Improve parentheses in code generation. https://github.com/hkust-taco/mlscript/blob/ba99965401408470e4072d95081199c504c004a8/shared/src/main/scala/mlscript/codegen/Codegen.scala#L415-L416
- Load built-in symbols from Typer. https://github.com/hkust-taco/mlscript/blob/ba99965401408470e4072d95081199c504c004a8/shared/src/main/scala/mlscript/codegen/Scope.scala#L23-L26 https://github.com/hkust-taco/mlscript/blob/ba99965401408470e4072d95081199c504c004a8/shared/src/main/scala/mlscript/codegen/Scope.scala#L45-L51
[x] Trait test in pattern matching has not been implemented. This can be done via Symbol.

Jan 11 '22 09:01 chengluyu

[ ] Trait test in pattern matching has not been implemented. This can be done via Symbol.

Traits can also introduce method definitions that can be inherited by classes, so they're not just for pattern matching!

Jan 12 '22 07:01 LPTK

[X] Implement tuple field accessors, as in (1,2,3)._1 – they currently return undefined

[EDIT: actually we'll do it the JS way in the new parser so this shouldn't be necessary]

Jan 14 '22 10:01 LPTK

[X] console.log does not seem to print anything in DiffTests (kind of fixed by @fo5for in https://github.com/hkust-taco/mlscript/commit/ecca9d8d5072723b575496dac151067292d5129e, using stderr instead)

[x] fields defined in traits are not used to initialize class fields in constructors

:js
trait X: { x: int }
class Y: X
  method MX = this.x
//│ Defined trait X
//│ Defined class Y
//│ Defined Y.MX: (Y & x) -> int
//│ // Prelude
//│ class Y {
//│   constructor(fields) {
//│   }
//│   get MX() {
//│     return this.x;
//│   }
//│ }
//│ // End of generated code

(Y {x = 1}).x
//│ res: 1
//│    = undefined

[x] this is captured by enclosing functions, giving it wrong semantics (#123):

:js
class A: { x: int }
  method MA = let _ = 1 in this.x
//│ Defined class A
//│ Defined A.MA: A -> int
//│ // Prelude
//│ class A {
//│   constructor(fields) {
//│     this.x = fields.x;
//│   }
//│   get MA() {
//│     return ((function (_) {
//│       return this.x;
//│     })(1));
//│   }
//│ }
//│ // End of generated code

(A { x = 1 }).MA
//│ res: int
//│ Runtime error:
//│   TypeError: Cannot read properties of undefined (reading 'x')

Jan 26 '22 06:01 LPTK

[x] Handling of the res running variable is not consistent with DiffTests:

42
//│ res: 42
//│    = 42

:js
def foo = "oops"
//│ // Query 1
//│ globalThis.foo = "oops";
//│ res = foo;
//│ // End of generated code
//│ foo: "oops"
//│    = 'oops'

res
//│ res: 42
//│    = 'oops'

Jan 28 '22 00:01 LPTK

[x] Fix syntax error of objects wrapped by lambdas, should be ((x) => ({t1: gt(1)(2), t2: 1})) (#124).

fun x -> {t1 = gt 1 2; t2 = 1}
//│ // Prelude
//│ function gt(x, y) {
//│   if (arguments.length === 2) {
//│     return x > y;
//│   }else {
//│     return (y) => x > y;
//│   }
//│ }
//│ let res;
//│ // Query 1
//│ res = ((x) => {
//│   t1: gt(1)(2),
//│   t2: 1
//│ });
//| // ((x) => {t1: gt(1)(2), t2: 1}) is illegal
//│ // End of generated code
//│ res: anything -> {t1: bool, t2: 1}
//│    = > undefined

Apr 23 '22 10:04 andongfan

@andongfan would you like to try and tackle some of this issues, including the new one?

@chengluyu Is the list of issues on this ticket up to date? I feel like a few have been solved already.

Apr 23 '22 14:04 LPTK

@andongfan would you like to try and tackle some of this issues, including the new one?

I think I can have a try :)

Apr 23 '22 15:04 andongfan

[x] Code execution for variables with primes is broken -- they return the result of the previous query (Fixed in #122)

x = 0
//│ x: 0
//│  = 0

y' = true
//│ y': true
//│   = 0

Jun 28 '22 05:06 fo5for

[x] Code execution for variables with primes is broken -- they return the result of the previous query
x = 0
//│ x: 0
//│  = 0

y' = true
//│ y': true
//│   = 0

:js
y' = true
//│ // Query 1
//│ globalThis["y'"] = true;
//│ res = y';
//│ // End of generated code
//│ y': true
//│   = 0

We didn't handle the case.

Since when can we use primes in the identifiers?

Jun 28 '22 19:06 chengluyu

They've always been supported in ML-family languages like OCaml and Haskell 😛

Jun 28 '22 19:06 LPTK

[X] Division of integer values yields a non-integer value. (fixed in https://github.com/hkust-taco/mlscript/commit/f0a16428e12899781274540ede4c1e5df439565c)

def foo = 1 / 2
//│ foo: int
//│    = 0.5

def foo = 1 / 0
//│ foo: int
//│    = Infinity

def foo = 0 / 0
//│ foo: int
//│    = NaN

// Javascript code generated from
// def foo = 1 / 2
const foo = 1 / 2;

// Expecting a truncation operation here. like:
const foo = Math.trunc(1 / 2);

Jun 30 '22 04:06 asiloh

@zhao-huang Good catch! The initial context in Typer.scala should have "/" -> numberBinOpTy instead of "/" -> intBinOpTy, where numberBinOpTy = fun(singleTup(DecType), fun(singleTup(DecType), DecType)(noProv))(noProv). Note that int <: number so 1 / 2 should be typeable as number.

Jun 30 '22 09:06 LPTK

[ ] Fix code generation undefined for blocks in let/ fun bindings.

:p
let rec f = 1
//│ |#let| |#rec| |f| |#=| |1|
//│ Parsed: let rec f = 1;
//│ Desugared: rec def f: 1
//│ f: 1
//│  = 1

// TODO FIX undefined: should `return 1`;
:p
:js
let rec f =
  1
//│ |#let| |#rec| |f| |#=|→|1|←|
//│ Parsed: let rec f = {1};
//│ Desugared: rec def f: {1}
//│ // Query 1
//│ globalThis.f5 = (() => {
//│   1;
//│ })();
//│ // End of generated code
//│ f: 1
//│  = undefined

Sep 09 '22 07:09 andongfan

[ ] Fix:

def update2 i arr = let a = arr[i] in arr[i] <- case a of undefined -> error, _ -> a
//│ update2: int -> MutArray[in 'a out 'a & ~undefined] -> unit
//│        = [Function: update2]

update2 0 ((mut 1,))
//│ Runtime error:
//│   TypeError: ((intermediate value) , []) is not a function

Dec 01 '22 04:12 LPTK

[ ] Remove the self kludge introduced in https://github.com/hkust-taco/mlscript/pull/123. It seems the simpler solution was to simply never generate function(){...} lambdas, preferring => lambdas instead, which do not have the stupid this capture semantics.

Mar 17 '23 04:03 LPTK

[ ] Cc @NeilKleistGao

:js
let x' = 1
//│ let x': 1
//│ // Prelude
//│ let res;
//│ class TypingUnit {}
//│ const typing_unit = new TypingUnit;
//│ // Query 1
//│ globalThis.x$ = 1;
//│ // End of generated code
//│ x'
//│    = 1

:js
class Foo(x': Int)
//│ class Foo(x': Int)
//│ // Prelude
//│ class TypingUnit1 {
//│   #Foo;
//│   constructor() {
//│   }
//│   get Foo() {
//│     const qualifier = this;
//│     if (this.#Foo === undefined) {
//│       class Foo {
//│         #x';
//│         constructor(x$) {
//│           this.#x' = x$;
//│         }
//│       static
//│         unapply(x) {
//│           return [x.#x'];
//│         }
//│       };
//│       this.#Foo = ((x') => Object.freeze(new Foo(x')));
//│       this.#Foo.class = Foo;
//│       this.#Foo.unapply = Foo.unapply;
//│     }
//│     return this.#Foo;
//│   }
//│ }
//│ const typing_unit1 = new TypingUnit1;
//│ globalThis.Foo = typing_unit1.Foo;
//│ // End of generated code
//│ Syntax error:
//│   Unexpected string

Sep 19 '23 15:09 LPTK

[x] It seems there's a diff-test bug where it fails to show/execute the last result here:

// * FIXME This should crash due to `error`,
// * but the crash is somehow swallowed and we get the result of the previous statement instead!
// * The same happens with any other side effect, like `log(...)`
// * Note: this doesn't happen if the last line is in a spearate diff-test block
:showRepl
fun foo(x) = error
let r = foo(1)
//│ fun foo: anything -> nothing
//│ let r: nothing
//│ ┌ Block at NuScratch.mls:21
//│ ├─┬ Prelude
//│ │ ├── Code
//│ │ │   function error() {
//│ │ │     throw new Error("an error was thrown");
//│ │ │   }
//│ │ │   let res;
//│ │ │   class TypingUnit {}
//│ │ │   const typing_unit = new TypingUnit;
//│ │ └── Reply
//│ │     undefined
//│ ├─┬ Query 1/2
//│ │ ├── Prelude: <empty>
//│ │ ├── Code:
//│ │ ├──   globalThis.foo = function foo(x) {
//│ │ ├──     return error();
//│ │ ├──   };
//│ │ ├── Intermediate: [Function: foo]
//│ │ └── Reply: [success] [Function: foo]
//│ └─┬ Query 2/2
//│   ├── Prelude: <empty>
//│   ├── Code:
//│   ├──   globalThis.r = foo(1);
//│   └── Reply: [runtime error] Error: an error was thrown
//│ r
//│   = [Function: foo]

CC @chengluyu

Sep 23 '23 03:09 LPTK

It seems there's a diff-test bug where it fails to show/execute the last result here: [...]

After a quick check, I found that the code for outputting type inference results and execution results in DiffTests is a bit different. I remember that the previous behavior was to output the execution result after each type result. When did this change? I'm still debugging.

Sep 23 '23 12:09 chengluyu

It probably changed because :NewDefs works quite differently from before. The entire typing unit is typed and run as a whole instead of statement by statement.

Sep 23 '23 12:09 LPTK

Removed bad solution.

Thanks for the pointer! We can temporarily fix this by replacing this line (https://github.com/hkust-taco/mlscript/blob/febbc0ba04c1b6b1821c86ddf9d5696d5fb05a30/shared/src/test/scala/mlscript/DiffTests.scala#L605) with

                case NuFunDef(_, nme, snme, tparams, bod) =>

Then,

// * FIXME This should crash due to `error`,
// * but the crash is somehow swallowed and we get the result of the previous statement instead!
// * The same happens with any other side effect, like `log(...)`
// * Note: this doesn't happen if the last line is in a spearate diff-test block
fun foo(x) = error
let v = foo(1)
//│ fun foo: anything -> nothing
//│ let v: nothing
//│ foo
//│     = [Function: foo]
//│ v
//│ Runtime error:
//│   Error: an error was thrown

I propose the PR to fix this.

Sep 23 '23 12:09 chengluyu

[x] Code like this doesn't work (Cc @NeilKleistGao):

// FIXME
fun y = 1
module Foo {
  fun x = y
}
//│ fun y: 1
//│ module Foo {
//│   fun x: 1
//│ }
//│ Code generation encountered an error:
//│   unresolved symbol y

// FIXME
module Foo {
  fun x = y
}
fun y = 1
//│ module Foo {
//│   fun x: 1
//│ }
//│ fun y: 1
//│ Code generation encountered an error:
//│   unresolved symbol y

I couldn't for the life of me quickly figure out what was going on here because the JS backend infrastructure is so damn opaque and undebuggable. For example there's no way to visualize what's going wrong in the scope graph. I really don't understand why you haven't integrated it with the tracing infrastructure @chengluyu @NeilKleistGao – it shouldn't be hard at all.

Sep 29 '23 16:09 LPTK

[ ] Somehow the renaming behavior between declared classes and declared functions is inconsistent when the name is reserved in JS. We rename the latter and not the former.

declare fun throw: anything -> nothing
//│ fun throw: anything -> nothing

throw(0); 0
//│ 0
//│ res
//│ Runtime error:
//│   ReferenceError: throw1 is not defined

declare class throw(arg: anything): nothing
//│ declare class throw(arg: anything): nothing

throw(0); 0
//│ 0
//│ res
//│ Runtime error:
//│   0

Oct 02 '23 15:10 LPTK

[ ] Somehow the renaming behavior between declared classes and declared functions is inconsistent when the name is reserved in JS. We rename the latter and not the former.
declare fun throw: anything -> nothing
//│ fun throw: anything -> nothing

throw(0); 0
//│ 0
//│ res
//│ Runtime error:
//│   ReferenceError: throw1 is not defined

declare class throw(arg: anything): nothing
//│ declare class throw(arg: anything): nothing

throw(0); 0
//│ 0
//│ res
//│ Runtime error:
//│   0

I guess this is another problem caused by the code in #186. We really need to refactor the code in JSTestBackend. I will do this after the paper submission.

Oct 03 '23 02:10 NeilKleistGao

[ ] Class names are not escaped, unlike function names

:js
fun f' = 0
//│ fun f': 0
//│ // Prelude
//│ let res;
//│ class TypingUnit {}
//│ const typing_unit = new TypingUnit;
//│ // Query 1
//│ globalThis.f$ = function f$() {
//│   return 0;
//│ };
//│ // End of generated code

:js
module F'
//│ module F'
//│ // Prelude
//│ class TypingUnit1 {
//│   #F';
//│   constructor() {
//│   }
//│   get "F'"() {
//│     const qualifier = this;
//│     if (this.#F' === undefined) {
//│       class F' {}
//│       this.#F' = new F'();
//│       this.#F'.class = F';
//│     }
//│     return this.#F';
//│   }
//│ }
//│ const typing_unit1 = new TypingUnit1;
//│ globalThis["F'"] = typing_unit1["F'"];
//│ // End of generated code
//│ Syntax error:
//│   Unexpected string

Oct 04 '23 10:10 LPTK

[x] Brackets in expressions are not handled properly Likely cause: JSBackend.scala:280

:js
1 - (2 - 3)
//│ Int
//│ // Prelude
//│ let res;
//│ class TypingUnit {}
//│ const typing_unit = new TypingUnit;
//│ // Query 1
//│ res = 1 - 2 - 3;
//│ // End of generated code
//│ res
//│     = -4```

Oct 08 '23 17:10 HarrisL2

[ ] Avoid shadowing previous results if the current code has codegen errors (see #186 )

r with { x: 1 }
//│ {x: 1}
//│ res
//│     = { x: 1 }

:e // TODO support
let r = new {}
//│ ╔══[ERROR] Unexpected type `anything` after `new` keyword
//│ ║  l.80: 	let r = new {}
//│ ╙──      	            ^^
//│ let r: error
//│ Code generation encountered an error:
//│   Unsupported `new` class term: Bra(rcd = true, Rcd())

:ge
r : Object
//│ Object
//│ Code generation encountered an error:
//│   unguarded recursive use of by-value binding r

:ge
r with { x: 1 }
//│ error & {x: 1}
//│ Code generation encountered an error:
//│   unguarded recursive use of by-value binding r

Nov 30 '23 14:11 NeilKleistGao

[ ] In the web demo (but NOT in diff-tests!!), expression 2 + 2 generates JS code +(2, 2), which returns 2. This might be since https://github.com/hkust-taco/mlscript/pull/204

Jan 12 '24 09:01 LPTK

[ ] Error in generated code for:

:js
module Ref { mut val value = 123; set value = 1 }
//│ module Ref extends B {
//│   fun get: 'A
//│   mut val value: 123 | 1
//│ }
//│ where
//│   'A :> 123 | 1
//│ // Prelude
//│ class TypingUnit47 {
//│   #Ref;
//│   constructor() {
//│   }
//│   get Ref() {
//│     const qualifier = this;
//│     if (this.#Ref === undefined) {
//│       class Ref extends B.class {
//│         #value;
//│         get value() { return this.#value; }
//│         set value(value) { return this.#value = value; }
//│         constructor() {
//│           super();
//│           this.#value = 123;
//│           const value = this.#value;
//│           void(value = 1);
//│         }
//│       }
//│       this.#Ref = new Ref();
//│       this.#Ref.class = Ref;
//│     }
//│     return this.#Ref;
//│   }
//│ }
//│ const typing_unit47 = new TypingUnit47;
//│ globalThis.Ref = typing_unit47.Ref;
//│ // End of generated code
//│ TEST CASE FAILURE: There was an unexpected runtime error
//│ Runtime error:
//│   TypeError: Assignment to constant variable.

Feb 19 '24 16:02 LPTK

[ ] While class instances are frozen on construction via Object.freeze, it seems modules are not.

Feb 19 '24 16:02 LPTK