Fix name duplication in code generation for new definition typing

Please `git cherrypick` this to a separate PR.

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

@@ -1407,7 +1407,7 @@ abstract class JSTestBackend extends JSBackend(allowUnresolvedSymbols = false) {
         val isByvalueRecIn = if (isByname) None else Some(true)
         val bodyIsLam = body match { case _: Lam => true case _ => false }
         val symb = symNme.map(_.name)
-        scope.declareValue(nme, isByvalueRecIn, bodyIsLam, symb)
+        scope.declareValue(nme, isByvalueRecIn, bodyIsLam, symb, true)
       case _ => ()
     }
 

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

@@ -313,18 +313,26 @@ class Scope(val name: Str, enclosing: Opt[Scope]) {
     symbol
   }
 
-  def declareValue(lexicalName: Str, isByvalueRec: Option[Boolean], isLam: Boolean, symbolicName: Opt[Str]): ValueSymbol = {
+  def declareValue(
+      lexicalName: Str,
+      isByvalueRec: Option[Boolean],
+      isLam: Boolean,
+      symbolicName: Opt[Str],
+      /** Workaround for the first pass traversal with new definition typing. */
+      forNewDefsDryRun: Bool = false
+  ): ValueSymbol = {
     val runtimeName = lexicalValueSymbols.get(lexicalName) match {
       // If we are implementing a stub symbol and the stub symbol did not shadow any other
       // symbols, it is safe to reuse its `runtimeName`.
-      case S(sym: StubValueSymbol) if !sym.shadowing => sym.runtimeName
-      case S(sym: BuiltinSymbol) if !sym.accessed    => sym.runtimeName
-      case _                                         => allocateRuntimeName(lexicalName)
+      case S(sym: StubValueSymbol) if !sym.shadowing   => sym.runtimeName
+      case S(sym: ValueSymbol) if sym.forNewDefsDryRun => sym.runtimeName
+      case S(sym: BuiltinSymbol) if !sym.accessed      => sym.runtimeName
+      case _                                           => allocateRuntimeName(lexicalName)
     }
-    val symbol = ValueSymbol(lexicalName, runtimeName, isByvalueRec, isLam)
+    val symbol = ValueSymbol(lexicalName, runtimeName, isByvalueRec, isLam, forNewDefsDryRun)
     register(symbol)
     symbolicName.foreach { symbolicName =>
-      register(ValueSymbol(symbolicName, runtimeName, isByvalueRec, isLam))
+      register(ValueSymbol(symbolicName, runtimeName, isByvalueRec, isLam, forNewDefsDryRun))
     }
     symbol
   }
@@ -350,10 +358,16 @@ class Scope(val name: Str, enclosing: Opt[Scope]) {
       allowEscape: Bool
   ): StubValueSymbol = {
     val symbol = lexicalValueSymbols.get(lexicalName) match {
+      // If the existing symbol is a value symbol, but the value symbol is
+      // declared in the dry-run of new definition typing, we can reuse the
+      // runtime name.
+      case S(valueSymbol: ValueSymbol) if valueSymbol.forNewDefsDryRun =>
+        StubValueSymbol(lexicalName, valueSymbol.runtimeName, false, previous)
       // If a stub with the same name has been defined, use the name.
-      case S(value) => StubValueSymbol(lexicalName, value.runtimeName, true, previous)
+      case S(symbol) => StubValueSymbol(lexicalName, symbol.runtimeName, true, previous)
       // Otherwise, we will allocate a new name.
-      case N => StubValueSymbol(lexicalName, allocateRuntimeName(lexicalName), false, previous)
+      case N =>
+        StubValueSymbol(lexicalName, allocateRuntimeName(lexicalName), false, previous)
     }
     register(symbol)
     symbolicName.foreach { symbolicName =>

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

@@ -45,13 +45,27 @@ sealed trait NuTypeSymbol { sym: TypeSymbol =>
   def isNested: Bool = qualifier.isDefined // is nested in another class/mixin/module
 }
 
-sealed class ValueSymbol(val lexicalName: Str, val runtimeName: Str, val isByvalueRec: Option[Boolean], val isLam: Boolean) extends RuntimeSymbol {
+sealed class ValueSymbol(
+    val lexicalName: Str,
+    val runtimeName: Str,
+    val isByvalueRec: Option[Boolean],
+    val isLam: Boolean,
+    /** Workaround for the first pass traversal with new definition typing. */
+    val forNewDefsDryRun: Boolean
+) extends RuntimeSymbol {
   override def toString: Str = s"value $lexicalName"
 }
 
 object ValueSymbol {
-  def apply(lexicalName: Str, runtimeName: Str, isByvalueRec: Option[Boolean], isLam: Boolean): ValueSymbol =
-    new ValueSymbol(lexicalName, runtimeName, isByvalueRec, isLam)
+  def apply(
+      lexicalName: Str,
+      runtimeName: Str,
+      isByvalueRec: Option[Boolean],
+      isLam: Boolean,
+      /** Workaround for the first pass traversal with new definition typing. */
+      forNewDefsDryRun: Boolean = false
+  ): ValueSymbol =
+    new ValueSymbol(lexicalName, runtimeName, isByvalueRec, isLam, forNewDefsDryRun)
 }
 
 sealed case class TypeAliasSymbol(

shared/src/test/diff/codegen/AuxiliaryConstructors.mls

@@ -464,11 +464,11 @@ n.ll
 //│ class TypingUnit19 {}
 //│ const typing_unit19 = new TypingUnit19;
 //│ // Query 1
-//│ globalThis.m1 = g(1);
+//│ globalThis.m = g(1);
 //│ // Query 2
-//│ globalThis.n1 = m1(2);
+//│ globalThis.n = m(2);
 //│ // Query 3
-//│ res = n1.ll;
+//│ res = n.ll;
 //│ // End of generated code
 //│ m
 //│   = [Function (anonymous)]
@@ -487,7 +487,7 @@ let mm = new M()
 //│ class TypingUnit21 {}
 //│ const typing_unit21 = new TypingUnit21;
 //│ // Query 1
-//│ globalThis.mm1 = new M.class();
+//│ globalThis.mm = new M.class();
 //│ // End of generated code
 //│ mm
 //│    = M {}

shared/src/test/diff/codegen/ConstructorStmt.mls

@@ -105,7 +105,7 @@ let aa = A(42)
 //│ class TypingUnit5 {}
 //│ const typing_unit5 = new TypingUnit5;
 //│ // Query 1
-//│ globalThis.aa1 = A(42);
+//│ globalThis.aa = A(42);
 //│ // End of generated code
 //│ aa
 //│    = A {}
@@ -119,7 +119,7 @@ aa
 //│ class TypingUnit6 {}
 //│ const typing_unit6 = new TypingUnit6;
 //│ // Query 1
-//│ res = aa1;
+//│ res = aa;
 //│ // End of generated code
 //│ res
 //│     = A {}
@@ -131,7 +131,7 @@ let ab = A(0)
 //│ class TypingUnit7 {}
 //│ const typing_unit7 = new TypingUnit7;
 //│ // Query 1
-//│ globalThis.ab1 = A(0);
+//│ globalThis.ab = A(0);
 //│ // End of generated code
 //│ ab
 //│    = A {}
@@ -408,9 +408,9 @@ www.add(42)
 //│ class TypingUnit15 {}
 //│ const typing_unit15 = new TypingUnit15;
 //│ // Query 1
-//│ globalThis.www1 = W();
+//│ globalThis.www = W();
 //│ // Query 2
-//│ res = www1.add(42);
+//│ res = www.add(42);
 //│ // End of generated code
 //│ www
 //│     = W {}

shared/src/test/diff/codegen/Nested.mls

@@ -78,9 +78,9 @@ bb.b
 //│ class TypingUnit1 {}
 //│ const typing_unit1 = new TypingUnit1;
 //│ // Query 1
-//│ globalThis.bb1 = A.B(A.a);
+//│ globalThis.bb = A.B(A.a);
 //│ // Query 2
-//│ res = bb1.b;
+//│ res = bb.b;
 //│ // End of generated code
 //│ bb
 //│    = B {}
@@ -241,9 +241,9 @@ ee.x
 //│ class TypingUnit5 {}
 //│ const typing_unit5 = new TypingUnit5;
 //│ // Query 1
-//│ globalThis.ee1 = D.createE(42);
+//│ globalThis.ee = D.createE(42);
 //│ // Query 2
-//│ res = ee1.x;
+//│ res = ee.x;
 //│ // End of generated code
 //│ ee
 //│    = E {}
@@ -361,13 +361,13 @@ gg.sum
 //│ class TypingUnit7 {}
 //│ const typing_unit7 = new TypingUnit7;
 //│ // Query 1
-//│ globalThis.es1 = E(1);
+//│ globalThis.es = E(1);
 //│ // Query 2
-//│ globalThis.fff1 = es1.F(2);
+//│ globalThis.fff = es.F(2);
 //│ // Query 3
-//│ globalThis.gg1 = fff1.G(3);
+//│ globalThis.gg = fff.G(3);
 //│ // Query 4
-//│ res = gg1.sum;
+//│ res = gg.sum;
 //│ // End of generated code
 //│ es
 //│    = E {}
@@ -559,11 +559,11 @@ i.x
 //│ class TypingUnit10 {}
 //│ const typing_unit10 = new TypingUnit10;
 //│ // Query 1
-//│ globalThis.jj1 = G.H.J(42);
+//│ globalThis.jj = G.H.J(42);
 //│ // Query 2
-//│ globalThis.i1 = jj1.ii(2);
+//│ globalThis.i = jj.ii(2);
 //│ // Query 3
-//│ res = i1.x;
+//│ res = i.x;
 //│ // End of generated code
 //│ jj
 //│    = J {}
@@ -666,9 +666,9 @@ j.i.x
 //│ class TypingUnit12 {}
 //│ const typing_unit12 = new TypingUnit12;
 //│ // Query 1
-//│ globalThis.j1 = H.J(42);
+//│ globalThis.j = H.J(42);
 //│ // Query 2
-//│ res = j1.i.x;
+//│ res = j.i.x;
 //│ // End of generated code
 //│ j
 //│   = J {}
@@ -757,11 +757,11 @@ ij.incY
 //│ const typing_unit13 = new TypingUnit13;
 //│ globalThis.I = typing_unit13.I;
 //│ // Query 1
-//│ globalThis.i3 = I(1);
+//│ globalThis.i1 = I(1);
 //│ // Query 2
-//│ globalThis.ij1 = i3.J(0);
+//│ globalThis.ij = i1.J(0);
 //│ // Query 3
-//│ res = ij1.incY;
+//│ res = ij.incY;
 //│ // End of generated code
 //│ i
 //│   = I {}
@@ -890,9 +890,9 @@ let n = J.N(2)
 //│ class TypingUnit15 {}
 //│ const typing_unit15 = new TypingUnit15;
 //│ // Query 1
-//│ globalThis.m1 = J.M();
+//│ globalThis.m = J.M();
 //│ // Query 2
-//│ globalThis.n1 = J.N(2);
+//│ globalThis.n = J.N(2);
 //│ // End of generated code
 //│ m
 //│   = M {}

shared/src/test/diff/codegen/New.mls

@@ -46,7 +46,7 @@ let c = C
 //│ class TypingUnit3 {}
 //│ const typing_unit3 = new TypingUnit3;
 //│ // Query 1
-//│ globalThis.c1 = C;
+//│ globalThis.c = C;
 //│ // End of generated code
 //│ c
 //│   = [class C]
@@ -93,7 +93,7 @@ let c = C
 //│ class TypingUnit8 {}
 //│ const typing_unit8 = new TypingUnit8;
 //│ // Query 1
-//│ globalThis.c3 = C;
+//│ globalThis.c1 = C;
 //│ // End of generated code
 //│ c
 //│   = [Function (anonymous)] {

shared/src/test/diff/codegen/NewMatching.mls

@@ -136,7 +136,7 @@ fun foo(s) =
 //│ // Query 1
 //│ globalThis.foo = function foo(s) {
 //│   return ((() => {
-//│     return s instanceof Some.class ? (([t]) => ((b) => b + t.x)(s21.value))(Some.unapply(s)) : 0;
+//│     return s instanceof Some.class ? (([t]) => ((b) => b + t.x)(s2.value))(Some.unapply(s)) : 0;
 //│   })());
 //│ };
 //│ // End of generated code

shared/src/test/diff/codegen/NewMutualRef.mls

@@ -0,0 +1,72 @@
+:NewDefs
+
+:js
+fun foo: Int -> Int
+fun foo = x => x + 1 // <- This will be added as a value symbol before translating `Bar`.
+class Bar {
+  fun calc(x) = foo(x)
+}
+//│ fun foo: Int -> Int
+//│ class Bar {
+//│   constructor()
+//│   fun calc: Int -> Int
+//│ }
+//│ fun foo: Int -> Int
+//│ // Prelude
+//│ let res;
+//│ class TypingUnit {
+//│   #Bar;
+//│   constructor() {
+//│   }
+//│   get Bar() {
+//│     const qualifier = this;
+//│     if (this.#Bar === undefined) {
+//│       class Bar {
+//│         constructor() {
+//│         }
+//│         calc(x) {
+//│           return foo(x);
+//│         }
+//│       };
+//│       this.#Bar = Bar;
+//│     }
+//│     return this.#Bar;
+//│   }
+//│ }
+//│ const typing_unit = new TypingUnit;
+//│ globalThis.Bar = typing_unit.Bar;
+//│ // Query 1 is empty
+//│ // Query 2
+//│ globalThis.foo = function foo(x) {
+//│   return x + 1;
+//│ };
+//│ // End of generated code
+
+// Note: This test case looks trivial but it was like:
+//
+// ```
+// :re
+// (new Bar()).calc(0)
+// //│ Int
+// //│ res
+// //│ Runtime error:
+// //│   ReferenceError: foo is not defined
+// ```
+//
+// My fix is a little bit hacky. The root of the problem is: when generating
+// code within a class, we need all top-level bindings to be accessible. This
+// part of implementation of new-definition-typing chose to declare all term
+// `NuFunDef` as `ValueSymbol` in advance, but this can lead to the fact that
+// the same symbol is declared multiple times, thus wasting some runtime names.
+// Consequently, the code that references these wasted runtime names are invalid.
+// 
+// Actually, I have a better solution, but it requires adjusting the order of
+// translation, and I don't have much time to spend on this at the moment. So，
+// my current fix is rather hacky. But I will complete this part after `PreTyper`
+// is finished, when I replacing the old Scope with the new Scope.
+//
+//                                                      Luyu Cheng on 2023/12/30
+(new Bar()).calc(0)
+//│ Int
+//│ res
+//│     = 1

shared/src/test/diff/codegen/NuReplHost.mls

@@ -33,7 +33,7 @@ let r = foo(1)
 //│ └─┬ Query 2/2
 //│   ├── Prelude: <empty>
 //│   ├── Code:
-//│   ├──   globalThis.r1 = foo(1);
+//│   ├──   globalThis.r = foo(1);
 //│   └── Reply: [runtime error] Error: an error was thrown
 //│ r
 //│ Runtime error:
@@ -44,7 +44,7 @@ r
 //│ nothing
 //│ res
 //│ Runtime error:
-//│   ReferenceError: r1 is not defined
+//│   ReferenceError: r is not defined