Fix handling of let bindings in type definitions

Disallow accessing them as fields and using them as implementations

compiler/shared/test/diff/NuScratch.mls → compiler/shared/test/diff/CompilerScratch.mls

@@ -1 +1,3 @@
 :NewDefs
+
+

shared/src/main/scala/mlscript/ConstraintSolver.scala

@@ -93,6 +93,10 @@ class ConstraintSolver extends NormalForms { self: Typer =>
           def handle(virtualMembers: Map[Str, NuMember]): Opt[FieldType] =
             virtualMembers.get(fld.name) match {
               case S(d: TypedNuFun) =>
+                if (d.fd.isLetOrLetRec)
+                  err(msg"Let binding '${d.name}' cannot tbe accessed as a field" -> fld.toLoc ::
+                    msg"Use a `val` declaration to make it a field" -> d.fd.toLoc ::
+                    Nil)
                 val ty = d.typeSignature
                 S(
                   if (d.fd.isMut) FieldType(S(ty), ty)(d.prov)

shared/src/main/scala/mlscript/NuTypeDefs.scala

@@ -425,7 +425,7 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
     def symbolicName: Opt[Str] = fd.symbolicNme.map(_.name)
     def toLoc: Opt[Loc] = fd.toLoc
     lazy val prov = TypeProvenance(toLoc, "member")
-    def isPublic = true // TODO
+    def isPublic: Bool = !fd.isLetOrLetRec
     lazy val typeSignature: ST =
       if (fd.isMut) bodyType
       else PolymorphicType.mk(level, bodyType)
@@ -553,6 +553,8 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
     val funSigs = MutMap.empty[Str, NuFunDef]
     val implems = decls.filter {
       case fd @ NuFunDef(_, nme, snme, tparams, R(rhs)) =>
+        if (fd.isLetOrLetRec)
+          err(s"`let` bindings must have a right-hand side", fd.toLoc)
         funSigs.updateWith(nme.name) {
           case S(s) =>
             err(s"A type signature for '${nme.name}' was already given", fd.toLoc)
@@ -985,7 +987,7 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
     lazy val (typedSignatures, funImplems) : (Ls[(NuFunDef, ST)], Ls[NuFunDef]) = decl match {
       case td: NuTypeDef => ctx.nest.nextLevel { implicit ctx =>
         val (signatures, rest) = td.body.entities.partitionMap {
-          case fd @ NuFunDef(N | S(false), nme, snme, tparams, R(rhs)) => // currently `val`s are encoded with `S(false)`
+          case fd @ NuFunDef(_, nme, snme, tparams, R(rhs)) if !fd.isLetOrLetRec =>
             L((fd, rhs))
           // TODO also pick up signature off implems with typed params/results
           case s => R(s)
@@ -1045,7 +1047,7 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
     lazy val allFields: Set[Var] = decl match {
       case td: NuTypeDef =>
         (td.params.getOrElse(Tup(Nil)).fields.iterator.flatMap(_._1) ++ td.body.entities.iterator.collect {
-          case fd: NuFunDef => fd.nme
+          case fd: NuFunDef if !fd.isLetOrLetRec => fd.nme
         }).toSet ++ inheritedFields ++ tparams.map {
           case (nme @ TypeName(name), tv, _) =>
             Var(td.nme.name+"#"+name).withLocOf(nme)
@@ -1280,13 +1282,18 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
                 }
                 if (!td.isDecl && td.kind =/= Trt && !td.isAbstract) {
                   toImplement.foreach { m =>
+                    def mkErr(postfix: Ls[Message -> Opt[Loc]]) = err(
+                      msg"Member `${m.name}` is declared (or its declaration is inherited) but is not implemented in `${
+                          td.nme.name}`" -> td.nme.toLoc ::
+                        msg"Declared here:" -> m.toLoc ::
+                        postfix)
                     implemsMap.get(m.name) match {
-                      case S(_) =>
-                      case N =>
-                        err(msg"Member `${m.name}` is declared (or its declaration is inherited) but is not implemented in `${
-                            td.nme.name}`" -> td.nme.toLoc ::
-                          msg"Declared here:" -> m.toLoc ::
+                      case S(impl) =>
+                        if (impl.isPrivate) mkErr(
+                          msg"Note: ${impl.kind.str} member `${m.name}` is private and cannot be used as a valid implementation" -> impl.toLoc ::
                           Nil)
+                      case N =>
+                        mkErr(Nil)
                     }
                   }
                 }
@@ -1309,27 +1316,35 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
                   println(s"Checking overriding for ${m} against ${sigMap.get(m.name)}...")
                   (m, sigMap.get(m.name)) match {
                     case (_, N) =>
-                    case (m: TypedNuTermDef, S(fun: TypedNuTermDef)) => fun match {
+                    case (m: TypedNuTermDef, S(sig: TypedNuTermDef)) => sig match {
                       // * If the implementation and the declaration are in the same class,
                       // * it does not require to be virtual.
-                      case _ if fun.isPrivate => () // * Private members are not actually inherited
-                      case td: TypedNuFun if (!td.fd.isVirtual && !clsSigns.contains(fun)) =>
-                        err(msg"${m.kind.str.capitalize} member `${m.name
-                            }` is not virtual and cannot be overridden" -> m.toLoc ::
-                          msg"Originally declared here:" -> fun.toLoc ::
+                      case _ if sig.isPrivate => () // * Private members are not actually inherited
+                      case td: TypedNuFun if (!td.fd.isVirtual && !clsSigns.contains(sig)) =>
+                        err(msg"${m.kind.str.capitalize} member '${m.name
+                            }' is not virtual and cannot be overridden" -> m.toLoc ::
+                          msg"Originally declared here:" -> sig.toLoc ::
                           Nil)
                       case p: NuParam if (!p.isVirtual && !clsSigns.contains(p)) =>
-                        err(msg"Inherited parameter named `${m.name
-                            }` is not virtual and cannot be overridden" -> m.toLoc ::
-                          msg"Originally declared here:" -> fun.toLoc ::
+                        err(msg"Inherited parameter named '${m.name
+                            }' is not virtual and cannot be overridden" -> m.toLoc ::
+                          msg"Originally declared here:" -> sig.toLoc ::
                           Nil)
                       case _ =>
+                        m match {
+                          case fun: TypedNuFun if (fun.fd.isLetOrLetRec) =>
+                            err(msg"Cannot implement ${m.kind.str} member '${m.name
+                                }' with a let binding" -> m.toLoc ::
+                              msg"Originally declared here:" -> sig.toLoc ::
+                              Nil)
+                          case _ =>
+                        }
                         val mSign = m.typeSignature
                         implicit val prov: TP = mSign.prov
-                        constrain(mSign, fun.typeSignature)
+                        constrain(mSign, sig.typeSignature)
                     }
                     case (_, S(that)) =>
-                      err(msg"${m.kind.str.capitalize} member `${m.name}` cannot override ${
+                      err(msg"${m.kind.str.capitalize} member '${m.name}' cannot override ${
                           that.kind.str} member of the same name declared in parent" -> td.toLoc ::
                         msg"Originally declared here:" -> that.toLoc ::
                         Nil)
@@ -1714,9 +1729,11 @@ class NuTypeDefs extends ConstraintSolver { self: Typer =>
                       overrideCheck(clsSigns, ifaceMembers, clsSigns)
                     }()
 
-                    implemCheck(impltdMems,
-                      (clsSigns.iterator ++ ifaceMembers.iterator)
-                      .distinctBy(_.name).filterNot(_.isImplemented).toList)
+                    trace(s"Checking signature implementations...") {
+                      implemCheck(impltdMems,
+                        (clsSigns.iterator ++ ifaceMembers.iterator)
+                        .distinctBy(_.name).filterNot(_.isImplemented).toList)
+                    }()
 
                     val allMembers =
                       (ifaceMembers ++ impltdMems).map(d => d.name -> d).toMap ++ typedSignatureMembers

shared/src/main/scala/mlscript/syntax.scala

@@ -215,7 +215,7 @@ final case class NuTypeDef(
   }
 
 final case class NuFunDef(
-  isLetRec: Opt[Bool], // None means it's a `fun`, which is always recursive; Some means it's a `let` or `val`
+  isLetRec: Opt[Bool], // None means it's a `fun`, which is always recursive; Some means it's a `let`/`let rec` or `val`
   nme: Var,
   symbolicNme: Opt[Var],
   tparams: Ls[TypeName],
@@ -232,7 +232,9 @@ final case class NuFunDef(
   val body: Located = rhs.fold(identity, identity)
   def kind: DeclKind = Val
   val abstractLoc: Opt[Loc] = None
-
+
+  def isLetOrLetRec: Bool = isLetRec.isDefined && !genField
+
   // If the member has no implementation, it is virtual automatically
   def isVirtual: Bool = virtualLoc.nonEmpty || rhs.isRight
 

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

@@ -173,15 +173,15 @@ t.x
 
 class Cont
 class MySumCont extends Cont {
-  let init(n) = n + 1
+  val init(n) = n + 1
   mut val pc = init(0)
 }
 //│ class Cont {
 //│   constructor()
 //│ }
 //│ class MySumCont extends Cont {
 //│   constructor()
-//│   let init: Int -> Int
+//│   val init: Int -> Int
 //│   mut val pc: Int
 //│ }
 
@@ -200,22 +200,21 @@ if f(false) is MySumCont then 1 else 2
 //│ res
 //│     = 1
 
-// * FIXME reject: `let`is not accessible here
 new MySumCont().init
 //│ Int -> Int
 //│ res
-//│     = undefined
+//│     = [Function (anonymous)]
 
 
 :pe
 :e
 :ng
 class Test(mut val x: Int)
 //│ ╔══[PARSE ERROR] Unexpected 'val' keyword in expression position
-//│ ║  l.213: 	class Test(mut val x: Int)
+//│ ║  l.212: 	class Test(mut val x: Int)
 //│ ╙──       	               ^^^
 //│ ╔══[ERROR] Unsupported field specification
-//│ ║  l.213: 	class Test(mut val x: Int)
+//│ ║  l.212: 	class Test(mut val x: Int)
 //│ ╙──       	                   ^
 //│ class Test()
 
@@ -228,13 +227,13 @@ module Foo { val x = 0 }
 :e
 fun test = set Foo.x = 1
 //│ ╔══[ERROR] Type mismatch in assignment:
-//│ ║  l.229: 	fun test = set Foo.x = 1
+//│ ║  l.228: 	fun test = set Foo.x = 1
 //│ ║         	           ^^^^^^^^^^^^^
 //│ ╟── member of type `0` is not mutable
-//│ ║  l.223: 	module Foo { val x = 0 }
+//│ ║  l.222: 	module Foo { val x = 0 }
 //│ ║         	                 ^^^^^
 //│ ╟── but it flows into assigned field with expected type `?x`
-//│ ║  l.229: 	fun test = set Foo.x = 1
+//│ ║  l.228: 	fun test = set Foo.x = 1
 //│ ╙──       	                  ^^
 //│ fun test: ()
 

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

@@ -57,9 +57,15 @@ class A(x0: Int) {
 //│ globalThis.A = typing_unit.A;
 //│ // End of generated code
 
-// FIXME: should be rejected
+:e
 A(0).x1
-//│ Int
+//│ ╔══[ERROR] Let binding 'x1' cannot tbe accessed as a field
+//│ ║  l.61: 	A(0).x1
+//│ ║        	    ^^^
+//│ ╟── Use a `val` declaration to make it a field
+//│ ║  l.5: 	  let x1 = x0 + 1
+//│ ╙──     	      ^^^^^^^^^^^
+//│ Int | error
 //│ res
 //│     = undefined
 //│ // Output
@@ -167,10 +173,10 @@ class B(x: Int, val y: Int)
 :e
 B(0, 0).x
 //│ ╔══[ERROR] Parameter 'x' cannot be accessed as a field
-//│ ║  l.168: 	B(0, 0).x
+//│ ║  l.174: 	B(0, 0).x
 //│ ║         	       ^^
 //│ ╟── Either make the parameter a `val` or access it through destructuring
-//│ ║  l.130: 	class B(x: Int, val y: Int)
+//│ ║  l.136: 	class B(x: Int, val y: Int)
 //│ ╙──       	        ^
 //│ Int | error
 //│ res
@@ -187,7 +193,7 @@ class C {
   constructor(val x: Int, y: Int)
 }
 //│ ╔══[ERROR] Cannot use `val` in constructor parameters
-//│ ║  l.187: 	  constructor(val x: Int, y: Int)
+//│ ║  l.193: 	  constructor(val x: Int, y: Int)
 //│ ╙──       	                  ^
 //│ class C {
 //│   constructor(x: Int, y: Int)
@@ -220,14 +226,14 @@ class C {
 :e
 fun f(val x: Int) = x + 1
 //│ ╔══[ERROR] Cannot use `val` in this position
-//│ ║  l.221: 	fun f(val x: Int) = x + 1
+//│ ║  l.227: 	fun f(val x: Int) = x + 1
 //│ ╙──       	          ^^^^^^
 //│ fun f: (x: Int) -> Int
 
 :pe
 (val x: 1)
 //│ ╔══[PARSE ERROR] Illegal position for field specification
-//│ ║  l.228: 	(val x: 1)
+//│ ║  l.234: 	(val x: 1)
 //│ ╙──       	     ^^^^
 //│ 1
 //│ res
@@ -237,10 +243,10 @@ fun f(val x: Int) = x + 1
 :e
 (val x: 1) =>
 //│ ╔══[PARSE ERROR] Unexpected end of input; an expression was expected here
-//│ ║  l.238: 	(val x: 1) =>
+//│ ║  l.244: 	(val x: 1) =>
 //│ ╙──       	             ^
 //│ ╔══[ERROR] Cannot use `val` in this position
-//│ ║  l.238: 	(val x: 1) =>
+//│ ║  l.244: 	(val x: 1) =>
 //│ ╙──       	     ^^^^
 //│ (x: 1) -> ()
 //│ res
@@ -249,18 +255,18 @@ fun f(val x: Int) = x + 1
 :e
 (val x: 1) => ()
 //│ ╔══[ERROR] Cannot use `val` in this position
-//│ ║  l.250: 	(val x: 1) => ()
+//│ ║  l.256: 	(val x: 1) => ()
 //│ ╙──       	     ^^^^
 //│ (x: 1) -> ()
 //│ res
 //│     = [Function: res]
 
 class D(x: Int) {
-  let x = 1
+  val x = 1
 }
 if D(42) is D(x) then x else 0
 //│ class D(x: Int) {
-//│   let x: 1
+//│   val x: 1
 //│ }
 //│ 0 | 1
 //│ res
@@ -278,11 +284,11 @@ if E(42) is E(x) then x else 0
 //│     = 2
 
 class F(val x: Int) {
-  let x = 3
+  val x = 3
 }
 F(0).x
 //│ class F(x: Int) {
-//│   let x: 3
+//│   val x: 3
 //│ }
 //│ 3
 //│ res

shared/src/test/diff/nu/BadClassInherit.mls

@@ -170,7 +170,7 @@ trait B { class X { fun g = 1 } }
 
 :e
 class C extends A, B
-//│ ╔══[ERROR] Class member `X` cannot override class member of the same name declared in parent
+//│ ╔══[ERROR] Class member 'X' cannot override class member of the same name declared in parent
 //│ ║  l.172: 	class C extends A, B
 //│ ║         	^^^^^^^^^^^^^^^^^^^^
 //│ ╟── Originally declared here:
@@ -197,7 +197,7 @@ class C extends A, B
 class C extends A {
   class X { fun g = 1 }
 }
-//│ ╔══[ERROR] Class member `X` cannot override class member of the same name declared in parent
+//│ ╔══[ERROR] Class member 'X' cannot override class member of the same name declared in parent
 //│ ║  l.197: 	class C extends A {
 //│ ║         	^^^^^^^^^^^^^^^^^^^
 //│ ║  l.198: 	  class X { fun g = 1 }