Skip to content

Patch to first-class function transformer#404

Merged
LPTK merged 7 commits intohkust-taco:hkmc2from
NeilKleistGao:symbol🐦
Mar 10, 2026

Hidden character warning

The head ref may contain hidden characters: "symbol\ud83d\udc26"
Merged

Patch to first-class function transformer#404
LPTK merged 7 commits intohkust-taco:hkmc2from
NeilKleistGao:symbol🐦

Conversation

@NeilKleistGao
Copy link
Member

@NeilKleistGao NeilKleistGao commented Mar 5, 2026

A patch to #387.

  • When calling a function defined in runtime.mls or globalThis, there is no need to throw an exception (due to the lack of symbol information), since we know calling such a function does not need to use the func.call form.
  • When handling class Foo(x); new Foo(x), there is no need to check the path of the instantiation, since it is not a call. The term symbol of Foo.class has a kind syntax.Fun, which will break the transformation.
  • Add missing symbols used in UCS
  • Also check the disamb symbol to avoid transforming class construction unexpectedly

@LPTK
Copy link
Contributor

LPTK commented Mar 5, 2026

  • When calling a function defined in runtime.mls or globalThis, there is no need to throw an exception (due to the lack of symbol information), since we know calling such a function does not need to use the func.call form.

How do you know this? In the future, the runtime will also be compiled with lambda transformation, as that's how we'll compile to WASM. So ideally, we should have a proper solution to this problem. Don't we have the correct symbol, here?

@LPTK
Copy link
Contributor

LPTK commented Mar 5, 2026

  • The term symbol of Foo.class has a kind syntax.Fun, which will break the transformation.

Isn't that a bug? Foo.class is clearly not a function. Can we fix that instead?

@NeilKleistGao
Copy link
Member Author

Don't we have the correct symbol, here?

No, we don't have symbols here.
e.g., runtime.Tuple.get is created by the following helper function

protected final def sel(p: Term, k: Ident): Term.SynthSel =
    (Term.SynthSel(p, k)(N, N): Term.SynthSel).resolve // the first `N` indicates the symbol

runtime.unreachable is generated by the following helper function:

lazy val unreachableFn =
    Select(Value.Ref(State.runtimeSymbol), Tree.Ident("unreachable"))(N)

@NeilKleistGao
Copy link
Member Author

Isn't that a bug? Foo.class is clearly not a function. Can we fix that instead?

No. The actual case is: when writing new Foo(0), we are referring to the construction function, instead of the class definition

Foo1 = function Foo(x) {
  return globalThis.Object.freeze(new Foo.class(x));
};

and the .class suffix is generated by JSBuilder.scala. So it does have a kind syntax.Fun.

@LPTK
Copy link
Contributor

LPTK commented Mar 6, 2026

No, we don't have symbols here.

The correct fix is to put the symbol there, not to special-case the handling of some symbols in core compiler logic...

@LPTK
Copy link
Contributor

LPTK commented Mar 6, 2026

when writing new Foo(0), we are referring to the construction function, instead of the class definition

No, this isn't referring to the construction function. It should be referring to the class. Can you please try to fix it?

By the way, are you running your pass before or after resolution? The latter would make more sense.

@NeilKleistGao
Copy link
Member Author

NeilKleistGao commented Mar 9, 2026

Can you please try to fix it?

The information is correct. I just need to retrieve it from the disamb field.

are you running your pass before or after resolution?

After.


Other problems are addressed (except for the unexpected failing in CI).

@LPTK
Copy link
Contributor

LPTK commented Mar 9, 2026

No, this isn't referring to the construction function. It should be referring to the class. Can you please try to fix it?

The information is correct.

I checked, and the base reference is to the BMS of the class, not to the "construction function". The BMS represents an overloaded definition, which includes both a term (the function term) and a class. The fact that the JS backend compiles the class as a member of the function is irrelevant and has no manifestation in the IR, so I think you were just confused about the semantics of BMSs.

@LPTK
Copy link
Contributor

LPTK commented Mar 9, 2026

except for the unexpected failing in CI

Yeah, working on fixing it now.

@LPTK
Copy link
Contributor

LPTK commented Mar 10, 2026

Please fix the conflict.

Copy link
Contributor

@LPTK LPTK left a comment

Choose a reason for hiding this comment

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

LGTM

@LPTK LPTK merged commit 1589084 into hkust-taco:hkmc2 Mar 10, 2026
1 of 2 checks passed
@LPTK LPTK deleted the symbol🐦 branch March 10, 2026 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants