Add `Scoped` blocks #356

ychenfo · 2025-11-27T20:39:47Z

This PR introduces Scoped blocks. ~~It is still a draft because there is a lot of clean up to be done.~~

Currently, this PR:

Only symbols that are known to be declared in the elaborated tree and symbols generated during lowering are put in the Scoped block
Does not update later passes to properly maintain the Scoped blocks
Does not generate nested Scoped blocks for if branches

This reverts commit cf9d78f.

This reverts commit 5f9e63c.

getting runtime errors of some thing undefined different match branch share the same `argumentn$` symbol, so maybe only nest scopes in patmat

This reverts commit 79bd913.

This reverts commit ab0d660.

This reverts commit 3108bac.

…fail

This reverts commit 12ec16e.

hkmc2/shared/src/test/mlscript/codegen/ScopedBlocks.mls

hkmc2/shared/src/test/mlscript/codegen/ScopedBlocksAndHandlers.mls

hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala

- no declared vars in `Scoped` blocks - pCtor also gets a `Scoped` block, which is merged with ctor's `Scoped` block in jsbuilder because in the js backend, parent ctor and ctor are in the same js block - fix scoped blocks for lambdas for shortcut boolean ops

LPTK

Ok, I think this is almost ready to merge. Thanks for your sustained work through this complicated change!

The nice thing is that this change also lets us avoid the needless assignment to undefined when writing let x without an initializer.

hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala

hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala

LPTK · 2025-12-05T16:33:32Z

hkmc2/shared/src/main/scala/hkmc2/codegen/js/JSBuilder.scala

+    case Scoped(syms, body) =>
+      blockPreamble(p.imports.map(_._1).toSeq ++ syms.toSeq) ->
+      (imps.mkDocument(doc" # ") :/: block(body, endSemi = false).stripBreaks)
+    case _ =>


Does it actually happen that we get a non-empty definedVarsNoScoped but there is no scope surrounding it? Sounds like if it does, it should be fixed. But we could fix it later (then, add a comment).

(Note: the comment is also not outdated.)

This is also related to lambda lifting. Some generated variables, like lambda, can only be handled by definedVarsNoScoped. I've left a comment as well.

LPTK · 2025-12-05T16:36:53Z

hkmc2/shared/src/main/scala/hkmc2/codegen/js/JSBuilder.scala


+  def blockPreamble(ss: Iterable[Symbol])(using Raise, Scope): Document =
+    // TODO document: mutable var assnts require the lookup
+    val vars = ss.filter(scope.lookup(_).isEmpty).toArray.sortBy(_.uid).iterator.map(l =>


With this PR, the ugly lookup should no longer be needed. I saw that removing it currently crashes the compilation tests. Any idea why?

I think this is mainly because in blockk function, we have val pre = blockPreamble(t.definedVarsNoScoped). Then t.definedVarsNoScoped will also cover symbols that have been collected by the outer Scoped object. Without the filtering will fail the assertion when we try to duplicate those symbols.

Why do we need to use t.definedVarsNoScoped, here? We should already know what is supposed to be in scope, thanks to Scoped.

Do you have a concrete example of this?

Why do we need to use t.definedVarsNoScoped, here? We should already know what is supposed to be in scope, thanks to Scoped.

Do you have a concrete example of this?

One example is the let done() = ... in mlscript-compile/Render.mls. If we do not call blockPreamble to allocate names, then this done will not be found later. I guess it is related to lambda lifting. Should we leave a comment here and handle it in another PR?

Should we leave a comment here and handle it in another PR?

Ok, let's do that.

Lambda lifting should be updated to deal with this later.

I've added a comment to track it.

hkmc2/shared/src/main/scala/hkmc2/codegen/js/JSBuilder.scala

LPTK · 2025-12-07T14:56:30Z

hkmc2/shared/src/test/mlscript/codegen/RandomStuff.mls

 //│   }
 //│ };

 :sjs


Where does this useless tmp come from? Why is it collected in the first place?

With :noTailRec, fun f = f is compiled to the following program, where tmp is the result of application.

:noTailRec :sjs fun f = f //│ JS (unsanitized): //│ let f; f = function f() { let tmp; tmp = f(); return tmp };

tmp is created and collected here.

Ok, fair enough. This will be better dealt with through a pass of dead-binding elimination.

hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala

Co-authored-by: Lionel Parreaux <[email protected]>

LPTK

Ok, I think this PR is now almost ready to be merged. Nice work on this major change of the IR!

But please address the comments I left first.

LPTK · 2025-12-08T03:54:22Z

hkmc2/shared/src/test/mlscript/codegen/Formatting.mls

 let discard = drop _
 //│ JS (unsanitized):
-//│ let discard, discard1; discard1 = function discard(_0) { return runtime.Unit }; discard = discard1;
+//│ let discard;


It looks like the discard variable is not properly collected.

One of the discard symbols is created by lambda rewriter, so it's indeed not collected now.

LPTK · 2025-12-08T03:55:19Z

hkmc2/shared/src/test/mlscript/codegen/Getters.mls

 //│ baz = function baz() {
-//│   let w, z, lambda;
+//│   let w, z;
+//│   let lambda;


Why is this lambda bound separately? Is it not properly collected?

This lambda is also created by the lambda rewriter and not collected now.

LPTK · 2025-12-08T03:55:46Z

hkmc2/shared/src/test/mlscript/codegen/Hygiene.mls

 f()
 //│ JS (unsanitized):
-//│ let x, f, x1, f1; x = 1; f1 = function f() { return x }; f = f1; x1 = 2; runtime.safeCall(f())
+//│ let x, f, x1; let f1; x = 1; f1 = function f() { return x }; f = f1; x1 = 2; runtime.safeCall(f())


Why is f2 bound separately? Is it not properly collected?

This f1 is also created by the lambda rewriter and not collected now.

Actually do we still want to generate Result.Lambda at all during lowering (since lambda rewriter is always on)? If we don't do so all these un-collected symbols related to lambdas should be gone.

Clearly, either the "lambda rewriter" should be fixed (which should be very easy; it's distinct from the lifter) or we should just lower directly to a function on the fly and remove lambda rewriting. I am not sure why we currently still have lambdas in values. I think it's a historical artifact.

Ah, I just noticed Lambda is already no longer a value. I guess it also does not need to be a Result...

Oh, so do we want to totally remove the Lambda form (since Lambdas don't need to be either Values or Results)...?

I guess in this PR it would be easier to only add fixes in LambdaRewriter.scala.

Ok, sure. Pls do it.

LPTK · 2025-12-08T03:57:27Z

hkmc2/shared/src/test/mlscript/codegen/NestedScoped.mls

There is too much :sjs output in this file. Please only keep one or two representative cases.

LPTK · 2025-12-08T03:57:57Z

hkmc2/shared/src/test/mlscript/codegen/Quasiquotes.mls

-//│ tmp6 = globalThis.Object.freeze(new Term.Symbol("x"));
-//│ x1 = globalThis.Object.freeze(new Term.Ref(tmp6));
-//│ tmp7 = Predef.print(x1);
+//│ let tmp6;


Why is this bound separately? Is it not properly collected?

This is because we have not handled temporary variables in quotations. I think I will do it in the next PR (this PR is already huge and complicated).

Ok, but FYI this PR is not really huge 👀

LPTK · 2025-12-08T03:59:50Z

hkmc2/shared/src/test/mlscript/codegen/ScopedBlocks.mls

There is too much :sjs output in this file. Please only keep one or two representative cases and those where the comments specifically talks about the generated code.

LPTK · 2025-12-08T04:01:30Z

hkmc2/shared/src/test/mlscript/ctx/MissingDefinitions2.mls

 val x

-:ge
+// with Scoped blocks, `x` is declared


This comment will be impossible to understand for someone else. Pls rephrase it.

Same remark for the blocks below.

LPTK · 2025-12-08T04:01:45Z

hkmc2/shared/src/test/mlscript/ctx/MissingDefinitions2.mls


-:ge
+// with Scoped blocks, `x` is declared
+// :ge


Why would we keep this commented line?

Same remark for the blocks below.

LPTK · 2025-12-08T04:05:05Z

hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala

          val l = new TempSymbol(S(ref))
+          loweringCtx.collectScopedSym(l)


The new TempSymbol + collectScopedSym pattern is all over the place. This is a time where you introduce a helper function (call it registerTempSymbol) to avoid the needless repetition. Keep it DRY!

NeilKleistGao and others added 30 commits November 11, 2025 17:01

Add scoped definition

bf9e386

WIP: Try to get symbols for scoped

887ca0b

wip

148006b

wip and some tests

b28875a

WIP: Try

5f9e63c

Try to use a pass to insert Scoped

cf9d78f

Revert "Try to use a pass to insert Scoped"

44f132f

This reverts commit cf9d78f.

Revert "WIP: Try"

07eff8f

This reverts commit 5f9e63c.

wip:

3108bac

getting runtime errors of some thing undefined different match branch share the same `argumentn$` symbol, so maybe only nest scopes in patmat

wip: add known bugs in the tests

ab0d660

wip wip

79bd913

Revert "wip wip"

c5d16e0

This reverts commit 79bd913.

Revert "wip: add known bugs in the tests"

69ad890

This reverts commit ab0d660.

Revert "wip:"

aeb0c5a

This reverts commit 3108bac.

Merge remote-tracking branch 'origin/hkmc2' into scope🥨

a9cbbf9

use collection.Set

021c1a0

wip: a single if in a nested block..?

0d4ae05

wip

f4ccdf5

many collection of symbols created during lowering

e796c84

forgot to collect some symbols and create some scoped blocks

95c3c12

minor

245b9d4

minor fix

214fc28

fix two tests

15f35be

hopefully complete Term.definedSyms

30c1ae9

wip: make all tests pass

46dd487

wip: meaningful use scoped var a bit... now tests related to handler …

046827a

…fail

wip: nested Scoped blocks for pattern matching bodies with bugs

12ec16e

Revert "wip: nested Scoped blocks for pattern matching bodies with bugs"

a11aa2d

This reverts commit 12ec16e.

wip: try to fix errors in handler tests

50253ae

wip: smart constructors..?

1871ea2

ychenfo added 6 commits December 5, 2025 00:04

fix scopes for nested if and floatOutUntilScope

1ac0ae9

Merge remote-tracking branch 'cunyuan/scope🥨' into scope🥨

30f3850

Merge remote-tracking branch 'origin/hkmc2' into scope🥨

d0e2164

try to add some comments to the entangled kludge

63fe1c4

update loop rewriting flag

43fdc78

some cleanup

08d4467

LPTK reviewed Dec 5, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/codegen/ScopedBlocks.mls Outdated Show resolved Hide resolved

LPTK reviewed Dec 5, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/codegen/ScopedBlocksAndHandlers.mls Outdated Show resolved Hide resolved

LPTK reviewed Dec 5, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala Outdated Show resolved Hide resolved

LPTK reviewed Dec 5, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala Outdated Show resolved Hide resolved

NeilKleistGao and others added 5 commits December 5, 2025 17:43

Change the form of scope.locally

9d6b59c

Fix the missing case

9f23fed

pr comments

70ed84a

cleanup and no "/** scoped **/"

8c11671

fixes

7dc143d

- no declared vars in `Scoped` blocks - pCtor also gets a `Scoped` block, which is merged with ctor's `Scoped` block in jsbuilder because in the js backend, parent ctor and ctor are in the same js block - fix scoped blocks for lambdas for shortcut boolean ops

ychenfo marked this pull request as ready for review December 5, 2025 12:07

Make some improvements

0227b70

LPTK requested changes Dec 5, 2025

View reviewed changes

NeilKleistGao and others added 5 commits December 6, 2025 10:18

Minor fix

5d99227

Minor

a6b1adb

Merge remote-tracking branch 'origin/hkmc2' into scope🥨

fbc8c13

update comments for definedVarsNoScoped

9231b1b

Add some comments

899ee24

LPTK reviewed Dec 7, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala Outdated Show resolved Hide resolved

Update hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala

2482b1e

Co-authored-by: Lionel Parreaux <[email protected]>

LPTK approved these changes Dec 8, 2025

View reviewed changes

LPTK and others added 3 commits December 8, 2025 18:28

Minor

06aa495

Fix flatten for nested scopes

7962e06

Changes from meeting

562f927

		val l = new TempSymbol(S(ref))
		loweringCtx.collectScopedSym(l)

Add Scoped blocks #356

Are you sure you want to change the base?

Add Scoped blocks #356

Uh oh!

Conversation

ychenfo commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LPTK left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

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!

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!

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!

Add `Scoped` blocks #356

Add `Scoped` blocks #356

ychenfo commented Nov 27, 2025 •

edited

Loading