Skip to content
This repository has been archived by the owner on Jul 12, 2024. It is now read-only.

Fix #44: Support non-native JS classes. #51

Merged
merged 5 commits into from
Apr 1, 2024

Conversation

sjrd
Copy link
Collaborator

@sjrd sjrd commented Mar 31, 2024

The handling of the IR Trees themselves are straightforward. The real work happens in WasmBuilder, and in particular in genCreateJSClassFunction. The main difficulty is that we need to decompose the JS constructor into three pieces, because the call to the super constructor must syntactically appear as such in the JavaScript source of the constructor. There is a big comment in the code that explains the decomposition.

@sjrd
Copy link
Collaborator Author

sjrd commented Mar 31, 2024

Rebased on top of #50.

sjrd added 3 commits April 1, 2024 10:49
In particular, we do not let the user of `WasmFunctionContext`
choose arbitrary `WasmLocalName`s anymore for receivers and local
definitions. They must all be chosen by the function context.

The names of parameters can still be chosen by the users for now.
In the process, we change the strategy to unpack capture data structs
in closure functions. Previously, we unpacked all the captures at the
start of the function. Now, we unpack them only when needed.

A function context now remembers its current environment, which is a
map from `LocalName` to `VarStorage`. A `VarStorage` can either be
an actual `Local` variable, or a `StructField`. The latter is used
for capture data structs.

This protects the choice of `WasmLocalName`s entirely inside
`WasmFunctionContext`.
Copy link
Owner

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! I left one question (+ some notes) but otherwise LGTM 👍

wasm/src/main/scala/ir2wasm/WasmBuilder.scala Show resolved Hide resolved
Comment on lines +590 to +598
val dataStructType = ctx.getClosureDataStructType(jsClassCaptures.map(_.ptpe))
val dataStructLocal = fctx.addLocal(
"__classCaptures",
WasmRefType(WasmHeapType.Type(dataStructType.name))
)
for (cc <- jsClassCaptures)
instrs += LOCAL_GET(fctx.lookupLocalAssertLocalStorage(cc.name.name))
instrs += STRUCT_NEW(dataStructType.name)
instrs += LOCAL_TEE(dataStructLocal)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note]
Construct a struct that contains jsClassCaptures and store into the local

     (local $__classCaptures___local (ref $captureData__5___struct)) ;; ...
     local.get $superClass$___local
     local.get $capture$1$2___local
     struct.new $captureData__5___struct
     local.tee $__classCaptures___local


// Load super constructor
val jsSuperClassTree = clazz.jsSuperClass.getOrElse {
IRTrees.LoadJSConstructor(clazz.superClass.get.name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[question]
Why we can use IRTrees.LoadJSConstructor(clazz.superClass.get.name) instead of jsSuperClass if its None?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that is the spec 🤷‍♂️
https://lampwww.epfl.ch/~doeraene/sjsir-semantics/#sec-sjsir-classdef-runtime-semantics-evaluation

I added a comment with a link to the spec and a summary of what it says.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

instrs += ctx.refFuncWithDeclaration(postSuperStatsFun.name)

// Call the createJSClass helper to bundle everything
instrs += CALL(FuncIdx(WasmFunctionName.createJSClass))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note]

  createJSClass: (data, superClass, preSuperStats, superArgs, postSuperStats) => {
    return class extends superClass {
      constructor(...args) {
        var preSuperEnv = preSuperStats(data, new.target, ...args);
        super(...superArgs(data, preSuperEnv, new.target, ...args));
        postSuperStats(data, preSuperEnv, new.target, this, ...args);
      }
    };
  },

instrs += ctx.refFuncWithDeclaration(closureFuncName)

instrs += I32_CONST(I32(if (restParam.isDefined) params.size else -1))
instrs += CALL(FuncIdx(WasmFunctionName.installJSMethod))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note]

  installJSMethod: (data, jsClass, isStatic, name, func, fixedArgCount) => {
    var target = isStatic ? jsClass : jsClass.prototype;
    var closure = fixedArgCount < 0
      ? (function(...args) { return func(data, this, ...args); })
      : (function(...args) { return func(data, this, ...args.slice(0, fixedArgCount), args.slice(fixedArgCount))});
    target[name] = closure;
  },

Comment on lines +444 to +445
if (clazz.jsClassCaptures.isEmpty)
genLoadJSClassFunction(clazz)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note]
class definitions with capture cannot be just load. They have to new every time because the constructor is defined based on the surrounding captures.

Where, jsClassCaptures are for example,

[note]

    def localJSClass(capture: Int): js.Dynamic = {
      class Local(s: String) extends MyClass(s) {
        def getCapture(): Int = capture
      }
      js.constructorOf[Local]
    }
List(
 
 ParamDef(LocalIdent(LocalName<superClass$>),NoOriginalName,AnyType,false),
 
 ParamDef(LocalIdent(LocalName<capture$1$2>),OriginalName(capture$1),IntType,false)
)

sjrd added 2 commits April 1, 2024 18:19
The handling of the IR Trees themselves are straightforward. The
real work happens in `WasmBuilder`, and in particular in
`genCreateJSClassFunction`. The main difficulty is that we need to
decompose the JS constructor into three pieces, because the call to
the super constructor must syntactically appear as such in the
JavaScript source of the constructor. There is a big comment in the
code that explains the decomposition.
Copy link
Owner

@tanishiking tanishiking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🎉

@tanishiking tanishiking merged commit 2172773 into tanishiking:main Apr 1, 2024
1 check passed
@sjrd sjrd deleted the non-native-js-classes branch April 1, 2024 16:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants