Unify patterns in UCS and UPS by chengluyu · Pull Request #336 · hkust-taco/mlscript

chengluyu · 2025-09-16T23:41:03Z

The Updated UCS and UPS Compilation Pipelines

Term: If now contains SimpleSplit, which represents splits using nested patterns. Nested patterns are represented by Pattern, which is also used by pattern definitions.
Elaborator: Since the elaboration of split is implemented by several mutually recursive methods, I placed them in a trait called SplitElaborator to control the length of Elaborator and maintain modularity.
- At this step, we report semantic errors that do not require context, such as mixing do and then, as well as some obvious unreachable cases, like having more branches after an else.
Lowering is now responsible for most of the work.
- SimpleSplits in UCS expressions are expanded to Splits using SplitCompiler, then normalized by Normalization, and finally lowered to Block.
- The naïve compilation (generation of unapply and unapplyStringPrefix is now done by calling SplitCompiler for each PatternDef in Lowering.
- The efficient compilation (done by Instantiator and Compiler) is invoked by SplitCompiler when it sees @compile annotated on patterns.
- At this step, we perform all the checks on arguments and parameters.

# Conflicts: # hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala # hkmc2/shared/src/main/scala/hkmc2/codegen/Lowering.scala # hkmc2/shared/src/main/scala/hkmc2/codegen/js/JSBuilder.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/Elaborator.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/Resolver.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/Term.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/ups/NaiveCompiler.scala # hkmc2/shared/src/test/mlscript-compile/Runtime.mjs # hkmc2/shared/src/test/mlscript/backlog/ToTriage.mls # hkmc2/shared/src/test/mlscript/codegen/ClassMatching.mls # hkmc2/shared/src/test/mlscript/codegen/MergeMatchArms.mls # hkmc2/shared/src/test/mlscript/handlers/NonLocalReturns.mls # hkmc2/shared/src/test/mlscript/handlers/RecursiveHandlers.mls # hkmc2/shared/src/test/mlscript/ucs/general/LogicalConnectives.mls # hkmc2/shared/src/test/mlscript/ups/examples/Record.mls

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

hkmc2/shared/src/test/mlscript/ucs/general/LogicalConnectives.mls

hkmc2/shared/src/test/mlscript/ucs/hygiene/CrossBranchCapture.mls

hkmc2/shared/src/main/scala/hkmc2/semantics/Elaborator.scala

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

hkmc2/shared/src/test/mlscript/ucs/examples/BinarySearchTree.mls

hkmc2/shared/src/test/mlscript/ucs/syntax/Else.mls

hkmc2/shared/src/test/mlscript/ups/specialization/SimpleLiterals.mls

# Conflicts: # hkmc2/shared/src/main/scala/hkmc2/codegen/Block.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/Term.scala # hkmc2/shared/src/main/scala/hkmc2/semantics/ucs/Desugarer.scala # hkmc2/shared/src/main/scala/hkmc2/syntax/Tree.scala # hkmc2/shared/src/test/mlscript/backlog/UCS.mls # hkmc2/shared/src/test/mlscript/codegen/Do.mls # hkmc2/shared/src/test/mlscript/codegen/FieldSymbols.mls # hkmc2/shared/src/test/mlscript/syntax/WeirdBrackets.mls # hkmc2/shared/src/test/mlscript/ucs/hygiene/HygienicBindings.mls # hkmc2/shared/src/test/mlscript/ucs/hygiene/PatVars.mls # hkmc2/shared/src/test/mlscript/ucs/patterns/AliasPattern.mls # hkmc2/shared/src/test/mlscript/ucs/syntax/NestedOpSplits.mls # hkmc2/shared/src/test/mlscript/ups/Future.mls # hkmc2/shared/src/test/mlscript/ups/examples/Record.mls # hkmc2/shared/src/test/mlscript/ups/parametric/EtaConversion.mls

hkmc2/shared/src/main/scala/hkmc2/semantics/SimpleSplit.scala

LPTK

Some minor preliminary comments.

hkmc2/shared/src/main/scala/hkmc2/semantics/ucs/FlatPattern.scala

hkmc2/shared/src/main/scala/hkmc2/semantics/Elaborator.scala

hkmc2/shared/src/test/mlscript/ups/examples/Record.mls

LPTK · 2025-11-03T00:58:02Z

hkmc2/shared/src/test/mlscript/ups/JoinPatterns.mls

 //│ = false

-pattern Exponential = ("a" | "b") ~ ("c" | "d") ~ ("e" | "f") // ~ ("g" | "h") ~ ("i" | "j") ~ ("k" | "l") ~ ("m" | "n") ~ ("o" | "p") ~ ("q" | "r") ~ ("s" | "t") ~ ("u" | "v") ~ ("w" | "x") ~ ("y" | "z")
+:todo


The code generated by this pattern will grow exponentially. It should be addressed in a future optimized string patterns PR.

Why not say as much in a comment?

By the way, I did not expect this code to grow exponentially even without compilation 🤔 How come?

By the way, I did not expect this code to grow exponentially even without compilation 🤔 How come?

Because it concatenates many disjunctions. For example, ("a" | "b") ~ ("c" | "d") ~ ("e" | "f") ~ ("g" | "h") ~ ("i" | "j") ~ ("k" | "l") has six disjunctions in a row, resulting in 2^6 = 64 branches.

The branch deduplication only works for the innermost consequents.

hkmc2/shared/src/test/mlscript/ups/examples/EvaluationContext.mls

hkmc2/shared/src/test/mlscript/ups/syntax/MixedParameters.mls

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

hkmc2/shared/src/test/mlscript/ups/examples/Record.mls

LPTK

Impressive work, thanks!

Let's finally get this merged ASAP (event hough I have not reviewed everything in detail due to lack of time).

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

hkmc2/shared/src/main/scala/hkmc2/semantics/Term.scala

hkmc2/shared/src/main/scala/hkmc2/semantics/Elaborator.scala

hkmc2/shared/src/test/mlscript/ucs/hygiene/CrossBranchCapture.mls

hkmc2/shared/src/test/mlscript/ucs/patterns/Compilation.mls

LPTK · 2025-11-07T06:20:07Z

hkmc2/shared/src/test/mlscript/ucs/patterns/ConjunctionPattern.mls

 object B
 data class C(a)

+// Note: It is normalization that caused the following tests to generate


Is this referring to a past incorrect behavior? Where is the problem, here?

Now. The current behavior is incorrect. The normalization assumes all classes are mutually exclusive, causing patterns like A & B to be optimized away instead of properly testing both classes.

I clarify the comment in caabab8.

Wow that's a huge miscompilation issue; should be front and center in the tracking issue!

[comment moved from the wrong place]

LPTK · 2025-11-07T06:21:34Z

package.json

@@ -1,4 +1,5 @@
 {
+  "name": "mlscript",


Do you know what caused this change?

I changed it in this commit: Keep name field in the lockfile stable. The reason is that npm copies the name field from package.json to the lockfile and will use the project folder's name if it's missing.

People might clone this repo to folders with different names and then run npm. So, it's necessary to keep this field to make the lockfile stable.

Ah, great! I've had this problem but didn't know it could be fixed this way 👍

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

hkmc2/shared/src/test/mlscript/ucs/patterns/ConjunctionPattern.mls

Copilot

Pull Request Overview

This PR refactors the representation of keyword-based infix operators in the parser by wrapping them in a Keywrd type instead of using raw Keyword.Infix values directly. This provides better type safety and consistency with how prefix keywords are handled.

Key Changes

Changed InfixApp constructor to use Keywrd[Keyword.Infix] instead of raw Keyword.Infix
Updated the parser to create Keywrd wrappers when constructing InfixApp nodes
Modified parse rules to use a more type-safe makeInfixRule approach
Added the package.json name field for the npm package
Updated numerous test expectations to reflect the new Keywrd wrapper in parse tree output

Reviewed Changes

Copilot reviewed 135 out of 136 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
package.json, package-lock.json	Added missing `name` field to package metadata
hkmc2/syntax/Tree.scala	Changed `InfixApp` to use `Keywrd[Keyword.Infix]` instead of bare `Keyword.Infix`
hkmc2/syntax/Parser.scala	Updated parser to create `Keywrd` wrappers for infix keywords
hkmc2/syntax/ParseRule.scala	Refactored infix rule generation to use type-safe `makeInfixRule` approach
hkmc2/semantics/BlockImpl.scala	Updated one location constructing `InfixApp` to use `Keywrd`
hkmc2DiffTests/.../DiffMaker.scala	Changed `==` to `===` for consistency
hkmc2DiffTests/.../BbmlDiffMaker.scala	Added `given` clause that was missing
Test files (*.mls)	Updated test expectations reflecting the parse tree changes
mlscript-compile/Runtime.mls, Runtime.mjs	Renamed `MatchResult` to `MatchSuccess` for clarity

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

Copilot · 2025-11-07T07:56:12Z

hkmc2/shared/src/test/mlscript-compile/Runtime.mjs

+            scrut1 = prevHandlerFrame.nextHandler.handler !== cur.handler;
+            if (scrut1 === true) {
+              prevHandlerFrame = prevHandlerFrame.nextHandler;
+              tmp = runtime.Unit;


The value assigned to tmp here is unused.

Wow, thanks Copilot!!!

chengluyu added 8 commits August 15, 2025 21:30

Sketch the NewDesugarer

5be4823

Prevent duplicate terms using Label in normalization

f8d040d

Fix that NewDesugarer cannot handle let bindings

18b9b7f

Check if errors from new and old desugarers are consistent

5a27c9e

Fix spacing after while and line break before break

0c15ac2

Desugar splits using NaiveCompiler and fix numerous issues

a079bd9

Deprecate Desugarer and fix related issues

8d77887

LPTK reviewed Sep 17, 2025

View reviewed changes

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

LPTK reviewed Sep 17, 2025

View reviewed changes

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

LPTK reviewed Sep 17, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ucs/general/LogicalConnectives.mls Show resolved Hide resolved

LPTK reviewed Sep 17, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ucs/hygiene/CrossBranchCapture.mls Show resolved Hide resolved

chengluyu added 5 commits September 17, 2025 13:00

Add the test from today's stand-up meeting

1b8617b

Deduplicate throw blocks that raise match errors

defdc53

Improve shorthands that were not sufficiently tested

e661386

Comment on a test which worked before

24e3f73

Separate symbols in patterns and terms to fix where clauses

ff24d0b

LPTK reviewed Sep 19, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/semantics/Elaborator.scala Show resolved Hide resolved

chengluyu and others added 3 commits September 20, 2025 12:11

Find dead splits after else & treat _ as else only in splits

0030549

Document a possible future work

0fb7066

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Avoid generating unnecessarybreak in the outermost Label

3ca6d55

LPTK reviewed Sep 20, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ucs/examples/BinarySearchTree.mls Show resolved Hide resolved

LPTK reviewed Sep 20, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ucs/syntax/Else.mls Show resolved Hide resolved

LPTK reviewed Sep 20, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ups/specialization/SimpleLiterals.mls Outdated Show resolved Hide resolved

chengluyu added 4 commits September 22, 2025 10:02

Add locations to the keyword of InfixApp

b80d4bc

Document the reason why we need two sets of symbols

28336d7

Support annotation in the desugarer

c58b4fd

LPTK reviewed Sep 22, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/semantics/SimpleSplit.scala Outdated Show resolved Hide resolved

LPTK reviewed Sep 22, 2025

View reviewed changes

hkmc2/shared/src/main/scala/hkmc2/semantics/SimpleSplit.scala Outdated Show resolved Hide resolved

LPTK reviewed Nov 6, 2025

View reviewed changes

chengluyu and others added 3 commits November 7, 2025 11:17

Update a comment

f49721a

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Merge branch 'luyu/refactoring-desugarer'

4074ce0

Fix parentheses in a test

0182701

LPTK reviewed Nov 7, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ups/examples/Record.mls Show resolved Hide resolved

chengluyu added 3 commits November 7, 2025 12:32

Document a precedence problem

17a201c

Use MutMap

571f84e

Add changes missing in the previous commit

274b5f9

LPTK approved these changes Nov 7, 2025

View reviewed changes

chengluyu and others added 15 commits November 7, 2025 14:26

Update a comment

29d221d

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Add tests & fix a couple of things

ca8a1e7

Update a comment

cab0586

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Update a comment

e94246e

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Update a comment

3af4454

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Fix a few minor issues

2dde445

Update a comment.

3e58e5f

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Update a comment

8f8f656

Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>

Polish a warning

62a116c

Remove an overload

ed8744a

Rename MatchResult to MatchSuccess

64dad63

Add a comment for class SimpleSplit

679d8b0

Add a test

8f51cd1

Remove an outdated comment an unused method

20ff59c

Properly document a test

caabab8

LPTK reviewed Nov 7, 2025

View reviewed changes

hkmc2/shared/src/test/mlscript/ucs/patterns/ConjunctionPattern.mls Show resolved Hide resolved

LPTK requested a review from Copilot November 7, 2025 07:53

Merge branch 'hkmc2' into refactoring-desugarer

5ae5df4

Copilot AI reviewed Nov 7, 2025

View reviewed changes

LPTK merged commit ea7dc6e into hkust-taco:hkmc2 Nov 7, 2025
1 check passed

LPTK deleted the refactoring-desugarer branch November 7, 2025 08:01

Conversation

chengluyu commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The Updated UCS and UPS Compilation Pipelines

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LPTK Nov 7, 2025 • 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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

chengluyu commented Sep 16, 2025 •

edited

Loading

LPTK Nov 7, 2025 •

edited

Loading