Skip to content

Deduplicate UCS splits using LetSplit and UseSplit#486

Open
chengluyu wants to merge 12 commits into
hkust-taco:hkmc2from
chengluyu:nu-let-split
Open

Deduplicate UCS splits using LetSplit and UseSplit#486
chengluyu wants to merge 12 commits into
hkust-taco:hkmc2from
chengluyu:nu-let-split

Conversation

@chengluyu
Copy link
Copy Markdown
Member

No description provided.

chengluyu and others added 12 commits April 12, 2026 18:48
# Conflicts:
#	hkmc2/shared/src/test/mlscript/block-staging/Functions.mls
#	hkmc2/shared/src/test/mlscript/codegen/MergeMatchArms.mls
#	hkmc2/shared/src/test/mlscript/ucs/general/LogicalConnectives.mls
#	hkmc2/shared/src/test/mlscript/ucs/normalization/Deduplication.mls
In `normalizeImpl`, when the alternative references the same scrutinee
as the current branch, the alternative is duplicated and specialized
separately in `+` and `-` modes. When both specializations agree, the
two duplicates could in principle be shared via a single `LetSplit`.
That detection is not yet implemented; mark the bail-out site with a
TODO and add `SpecializedSplitSharing.mls` covering both:

- cases where the duplication is wasteful (disjoint sibling classes
  like `Cat()`/`Dog()` — both specializations agree, sharing would
  suffice);
- cases where the duplication is genuine (refining patterns like
  `S(0)`/`S(_)` — positive and negative specialization yield
  different splits, sharing would be incorrect).

The latter group guards against a future over-eager sharing rule.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The `Split.Else` branch of the `++` extension was annotated as
impossible but silently discarded `those`. Replace the comment with a
`softAssert(false)` so that the invariant is enforced at runtime and
visible in stack traces if it ever breaks.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`JoinPointCtx` tracks the pending set of `SplitSymbol`s awaiting a
`LetSplit` placement — a piece of state genuinely local to the walk.
Stashing the sharing threshold in the same case class forced a
plumbing trick where an inner `normalize` call had to reconstruct the
context with `JoinPointCtx.withThreshold` just to carry a config
option across an invocation boundary.

The threshold is the only `Config` field `normalize` needs, and the
caller already has `Config` in scope. Just add `Config` to the class
constructor's `using` clause so all methods can read it via the
global `config` helper, drop the parameter from `apply`, and simplify
`JoinPointCtx` back to the symbol set it was originally.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@LPTK
Copy link
Copy Markdown
Contributor

LPTK commented May 10, 2026

Pls fix the conflicts.

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