Implement instance methods by b-temirov · Pull Request #429 · hkust-taco/mlscript

b-temirov · 2026-03-30T05:28:41Z

This PR adds class-method support for non-inheriting classes by lowering instance methods to standalone direct-call wasm functions with an explicit this parameter, and a prescan that predeclares constructors and methods so self recursion, mutual recursion, later-method calls, and constructor calls to same or later classes work by construction.

LPTK · 2026-03-30T05:50:20Z

hkmc2/shared/src/test/mlscript/wasm/ScopedLocals.mls

-//│               (i32.const 42))
-//│             (ref.i31
-//│               (i32.const 1))))
+//│           (ref.cast (ref $Foo)


Why is this new cast needed?

Previously we did

val objType = ctx.getFuncInfo_!(ctorFuncIdx).body.resultType_! call(funcidx = ctorFuncIdx, as.map(argument), Seq(Result(objType.asValType_!)))

in Instantiate(...) and used the body result type for the call. But now we are using predeclared ConstructorInfo, which doesn't include body result type, so the call using RefType.anyref is less precise, hence the cast

private def directConstructorCall( ctorInfo: ConstructorInfo, args: Seq[Expr], )(using Ctx): Expr = ref.cast( call( funcidx = FuncIdx(ctorInfo.funcId), operands = args, returnTypes = Seq(Result(RefType.anyref)), ), RefType(ctx.getType_!(ctorInfo.classBms), nullable = false), )

But now we are using predeclared ConstructorInfo, which doesn't include body result type

Why not?

We register ConstructorInfo in the prescan before body lowering so methods can reference constructors defined later in code. At that stage there is no emitted constructor body to get the result. But anyways removing cast seems to be fine.

hkmc2/shared/src/test/mlscript/wasm/Methods.mls

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

LPTK · 2026-04-01T04:35:42Z

Is this ready for review/merge?

b-temirov · 2026-04-01T10:35:02Z

Is this ready for review/merge?

Not quite. Inheritance coming soon on top. Or should I raise inheritance separately?

LPTK · 2026-04-01T10:54:39Z

Yes, better keep the PRs small and merge them regularly.

Copilot

Pull request overview

Adds WebAssembly backend support for instance/class methods on (non-inheriting) classes by lowering methods into standalone direct-call Wasm functions with an explicit receiver (this) parameter, plus a prescan that predeclares constructors/methods to support recursion and forward references.

Changes:

Add prescan metadata registration for constructors and methods, and emit lowered method functions.
Route constructor/method calls through direct call to the lowered functions (instead of indirect call_ref) where applicable.
Add/adjust Wasm golden tests for methods and for constructor calls in matching output.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
hkmc2/shared/src/test/mlscript/wasm/Methods.mls	New golden tests covering getters, nullary/callable methods, recursion, forward references, chaining, mutation, and error cases.
hkmc2/shared/src/test/mlscript/wasm/Matching.mls	Update golden WAT output to reflect direct constructor calls (`call`) rather than `call_ref`.
hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala	Implement method/ctor metadata prescan, emit lowered method funcs, and add direct-call lowering for constructors/methods and related selection/assignment handling.
hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/Ctx.scala	Add `MethodInfo`/`ConstructorInfo` metadata storage and accessors in the Wasm codegen context.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

LPTK

The logic seems quite complicated, for what this achieves. But let's try to merge this soon anyway, as soon as you have fixed what I point out.

LPTK · 2026-04-01T13:40:59Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

+  private def registerDefnCallableMetadata(b: Block)(using Ctx, Raise, Scope): Unit = b match
+    case Define(defn: ClsLikeDefn, rst) =>
+      registerClassCallableMetadata(defn)
+      registerDefnCallableMetadata(rst)
+    case Define(_, rst) =>
+      registerDefnCallableMetadata(rst)
+    case Match(_, arms, dflt, rst) =>
+      arms.foreach((_, body) => registerDefnCallableMetadata(body))
+      dflt.foreach(registerDefnCallableMetadata)
+      registerDefnCallableMetadata(rst)
+    case Begin(_, rst) =>
+      registerDefnCallableMetadata(rst)
+    case TryBlock(_, _, rst) =>
+      registerDefnCallableMetadata(rst)
+    case Assign(_, _, rst) =>
+      registerDefnCallableMetadata(rst)
+    case AssignField(_, _, _, rst) =>
+      registerDefnCallableMetadata(rst)
+    case AssignDynField(_, _, _, _, rst) =>
+      registerDefnCallableMetadata(rst)
+    case HandleBlock(_, _, _, _, _, _, _, rst) =>
+      registerDefnCallableMetadata(rst)
+    case Label(_, _, body, rst) =>
+      registerDefnCallableMetadata(body)
+      registerDefnCallableMetadata(rst)
+    case Scoped(_, body) =>
+      registerDefnCallableMetadata(body)
+    case _: BlockTail => ()


Wouldn't this work?

Suggested change

private def registerDefnCallableMetadata(b: Block)(using Ctx, Raise, Scope): Unit = b match

case Define(defn: ClsLikeDefn, rst) =>

registerClassCallableMetadata(defn)

registerDefnCallableMetadata(rst)

case Define(_, rst) =>

registerDefnCallableMetadata(rst)

case Match(_, arms, dflt, rst) =>

arms.foreach((_, body) => registerDefnCallableMetadata(body))

dflt.foreach(registerDefnCallableMetadata)

registerDefnCallableMetadata(rst)

case Begin(_, rst) =>

registerDefnCallableMetadata(rst)

case TryBlock(_, _, rst) =>

registerDefnCallableMetadata(rst)

case Assign(_, _, rst) =>

registerDefnCallableMetadata(rst)

case AssignField(_, _, _, rst) =>

registerDefnCallableMetadata(rst)

case AssignDynField(_, _, _, _, rst) =>

registerDefnCallableMetadata(rst)

case HandleBlock(_, _, _, _, _, _, _, rst) =>

registerDefnCallableMetadata(rst)

case Label(_, _, body, rst) =>

registerDefnCallableMetadata(body)

registerDefnCallableMetadata(rst)

case Scoped(_, body) =>

registerDefnCallableMetadata(body)

case _: BlockTail => ()

private def registerDefnCallableMetadata(b: Block)(using Ctx, Raise, Scope): Unit = b match

case Define(defn: ClsLikeDefn, rst) =>

registerClassCallableMetadata(defn)

registerDefnCallableMetadata(rst)

case _ => b.subBlocks.foreach(subBlocks)

Please also update createDefnTypes similarly.

LPTK · 2026-04-01T13:50:34Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

+        case _: BlockMemberSymbol => disamb.getOrElse(l)
+        case sym => sym
+      resolved match
+        case _: ClassSymbol | _: ModuleOrObjectSymbol => true


Wouldn't this trigger in

:lot object Test Test //│ —————————————| Lowered IR Tree |———————————————————————————————————————————————————————————————————— //│ Program: //│ main = Scoped(syms = {member:Test⁰}): //│ body = Define: //│ defn = ClsLikeDefn: //│ isym = object:Test¹ //│ sym = member:Test⁰ //│ k = Obj //│ rest = Return: \ //│ res = Ref{disamb=object:Test¹}: //│ l = member:Test⁰ //│ disamb = S of object:Test¹ //│ implct = true //│ —————————————————| Output |—————————————————————————————————————————————————————————————————————————

?

Again, self-references never have a disamb and always contain the inner symbol directly.

LPTK · 2026-04-01T13:50:54Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

+    sel.symbol.flatMap(_.asTrm).flatMap(ctx.getMethodInfo)
+
+  /** Returns whether `path` is an explicit owner reference such as a class or module path. */
+  private def isDirectOwnerRef(path: Path): Bool = path match


Is this supposed to be named

Suggested change

private def isDirectOwnerRef(path: Path): Bool = path match

private def isSelfReference(path: Path): Bool = path match

?

LPTK · 2026-04-01T13:53:11Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

+          val ctorCall = constructorInfoForPath(fun).map: ctorInfo =>
+            directConstructorCall(ctorInfo, args.map(argument))
+
+          val methodCall = fun match


This will always be computed, even when ctorCall is defined! You should move this expensive code to the orElse argument or make this val a def.

LPTK · 2026-04-01T13:55:18Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

-            )
+            lhs match
+              case ownerRef: Value.Ref if isDirectOwnerRef(ownerRef) =>
+                val resolvedOwner = ownerRef.l match


For the same reason stated above, I don't think this makes a lot of sense.

LPTK · 2026-04-01T13:56:07Z

hkmc2/shared/src/test/mlscript/wasm/Methods.mls

+//│ = 41
+
+
+:wat


We already have the :wat above, which I think is sufficient.

Suggested change

:wat

LPTK · 2026-04-01T13:57:54Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala

+              case S(fieldSym) =>
+                if ctx.getMethodInfo(fieldSym).nonEmpty then
+                  errExpr(
+                    Ls(msg"WatBuilder::returningTerm for AssignField(...) to class methods is not implemented yet" -> nme.toLoc),


I don't think we ever want to allow assigning to an object method. This error should be a standard compilation error, not a "not implemented yet".

Derppening

Just a few preliminary comments for now. I will need to look into this in more detail.

Derppening · 2026-04-01T15:15:11Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/Ctx.scala

+  enum MethodShape:
+    case Getter
+    case Callable


I didn't know there is a meaningful difference between bracketless vs bracketed invocations. AFAICT it seems like bracketless invocations are statically resolved and bracketed invocations are dynamically resolved.

@LPTK is this intended behavior?

The resolution is the same. It's just the difference between taking an argument list, as in x.f(), and not taking one, as in x.f (the latter is declared as fun f = ...).

TODO: we don't really need this enum; just store the number of parameter lists that each method accepts.

Derppening · 2026-04-01T15:15:26Z

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/Ctx.scala

+  case class MethodInfo(
+      ownerIsym: InnerSymbol,
+      dSym: TermSymbol,
+      funcId: SymIdx,
+      shape: MethodShape,
+  )
+
+  case class ConstructorInfo(
+      classBms: BlockMemberSymbol,
+      funcId: SymIdx,
+      exportName: Opt[Str],
+  )


Nit: Please add missing documentation.

b-temirov added 2 commits March 29, 2026 10:21

Add wasm class methods

a44a9d1

Merge remote-tracking branch 'taco/hkmc2' into vmethods

05491be

LPTK reviewed Mar 30, 2026

View reviewed changes

hkmc2/shared/src/test/mlscript/wasm/Methods.mls Outdated Show resolved Hide resolved

LPTK reviewed Mar 30, 2026

View reviewed changes

hkmc2/shared/src/test/mlscript/wasm/Methods.mls Show resolved Hide resolved

Add test cases

e3f5c69

LPTK reviewed Mar 30, 2026

View reviewed changes

hkmc2/shared/src/test/mlscript/wasm/Methods.mls Outdated Show resolved Hide resolved

Allow class field modification inside methods

361de21

LPTK reviewed Mar 30, 2026

View reviewed changes

hkmc2/shared/src/test/mlscript/wasm/Methods.mls Outdated Show resolved Hide resolved

LPTK reviewed Mar 30, 2026

View reviewed changes

hkmc2/shared/src/test/mlscript/wasm/Methods.mls Outdated Show resolved Hide resolved

b-temirov added 2 commits March 31, 2026 01:32

Fix error messages

5d3a089

Merge remote-tracking branch 'taco/hkmc2' into vmethods

74b886c

LPTK reviewed Mar 31, 2026

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala Outdated Show resolved Hide resolved

b-temirov and others added 2 commits April 1, 2026 02:39

Simplify duplicated error messages

ac5a835

Merge branch 'hkmc2' into vmethods

5370c5b

b-temirov marked this pull request as ready for review April 1, 2026 10:58

Copilot AI review requested due to automatic review settings April 1, 2026 10:58

Copilot started reviewing on behalf of b-temirov April 1, 2026 10:59 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala Outdated Show resolved Hide resolved

hkmc2/shared/src/main/scala/hkmc2/codegen/wasm/text/WatBuilder.scala Show resolved Hide resolved

LPTK and others added 2 commits April 1, 2026 20:24

Merge branch 'hkmc2' into vmethods

b5497e5

Apply small fixes

08f5e40

LPTK requested changes Apr 1, 2026

View reviewed changes

LPTK requested a review from Derppening April 1, 2026 14:00

Derppening reviewed Apr 1, 2026

View reviewed changes

Changes from meeting

bd4f8f8

	private def isDirectOwnerRef(path: Path): Bool = path match
	private def isSelfReference(path: Path): Bool = path match

Conversation

b-temirov commented Mar 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b-temirov Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LPTK commented Apr 1, 2026

Uh oh!

b-temirov commented Apr 1, 2026

Uh oh!

LPTK commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

LPTK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Derppening left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LPTK Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

b-temirov Mar 30, 2026 •

edited

Loading

LPTK Apr 1, 2026 •

edited

Loading