JavaScript code generation backlog

[chengluyu]

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

• Implement Class.Member instance calling convention. Source This syntax might be changed in the new parser.

• Add timeout to stream.read() otherwise we will occasionally suffer from infinite loops. Source (Fixed in Add timeout to prevent rogue tests from running forever #126)

• Automatically check references to unimplemented variables. Source

• Support merging arrays in withConstruct. Source TBD. The semantics of with might be changed.

• Some // TODO introduced in commits from Test JavaScript code generation #53. Source TBD.

  As for 1 Aug, 2022. The // TODOs are as follows. They are not urgent at this moment.

  • Translation of Blk.

    mlscript/shared/src/main/scala/mlscript/JSBackend.scala

    Lines 177 to 178 in ba99965

    	// TODO: find out if we need to support this.
    	case _: Def | _: TypeDef => throw CodeGenError("unexpected definitions in blocks")

  • Consider traits when sorting classes.

    mlscript/shared/src/main/scala/mlscript/JSBackend.scala

    Lines 482 to 485 in ba99965

    	topLevelScope.getType(name) match {
    	case S(sym: ClassSymbol) => S(sym)
    	case S(sym: TraitSymbol) => N // TODO: inherit from traits
    	case S(sym: TypeAliasSymbol) =>

  • Improve parentheses in code generation.

    mlscript/shared/src/main/scala/mlscript/codegen/Codegen.scala

    Lines 415 to 416 in ba99965

    	// TODO: Figure out how `=>` competes with other operators.
    	case L(expr: JSRecord) => expr.toSourceCode.parenthesized

  • Load built-in symbols from Typer.

    mlscript/shared/src/main/scala/mlscript/codegen/Scope.scala

    Lines 23 to 26 in ba99965

    	def this(name: Str) = {
    	this(name, N)
    	// TODO: allow types and values to have the same name
    	// TODO: read built-in symbols from `Typer`.

    mlscript/shared/src/main/scala/mlscript/codegen/Scope.scala

    Lines 45 to 51 in ba99965

    	// TODO: add `true`, `false`, and `error` to this list
    	register(TypeAliasSymbol("anything", Nil, Top))
    	register(TypeAliasSymbol("nothing", Nil, Bot))
    	// TODO: register them in the same way as `Typer` does.
    	Ls("int", "number", "bool", "string", "unit") foreach { name =>
    	register(TypeAliasSymbol(name, Nil, TypeName(name)))
    	}

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

[LPTK]

• 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!

[LPTK]

• 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]

[LPTK]

• console.log does not seem to print anything in DiffTests (kind of fixed by @fo5for in ecca9d8, using stderr instead)

• 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

• this is captured by enclosing functions, giving it wrong semantics (Fix this being captured by immediately invoked functions #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')

[LPTK]

• 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'

[andongfan]

• Fix syntax error of objects wrapped by lambdas, should be ((x) => ({t1: gt(1)(2), t2: 1})) (Add parentheses to records in lambdas #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

[LPTK]

@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.

[andongfan]

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

I think I can have a try :)

[fo5for]

• Code execution for variables with primes is broken -- they return the result of the previous query (Fixed in Fix incorrect code generation due to single quotes in identifiers #122)

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

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

[chengluyu]

• 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?

[LPTK]

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

[zhao-huang]

• Division of integer values yields a non-integer value. (fixed in f0a1642)

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);

[LPTK]

@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.

[andongfan]

• 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

[LPTK]

• 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

[LPTK]

• Remove the self kludge introduced in Fix this being captured by immediately invoked functions #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.

[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

[LPTK]

• 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

[chengluyu]

• 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.

[LPTK]

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.

[chengluyu]

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.

[LPTK]

• 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.

[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

[NeilKleistGao]

• 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.

[LPTK]

• 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

[HarrisL2]

• 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```

[NeilKleistGao]

• Avoid shadowing previous results if the current code has codegen errors (see Fix codegen error for global functions #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

[LPTK]

• In the web demo (but NOT in diff-tests!!), expression 2 + 2 generates JS code +(2, 2), which returns 2.
  This might be since Introduce comma operator and use ,/; instead of ;/;; #204

[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.

[LPTK]

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