Skip to content

Eliminate Reusing Mutable Terms #317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: hkmc2
Choose a base branch
from

Conversation

FlandiaYingman
Copy link

The current compiler assumes that all Term are immutable and is reusing them. This assumption is wrong because of the resolution stage. This PR tries to fix the places where terms are reused.

See also #297 (comment).

AFAIK there are still two places where terms are reused and I couldn't fix them.

  • During UCS desurgaring, the desugarer reuses the fallback split (e.g., in hkmc2/shared/src/test/mlscript/ucs/syntax/PlainConditionals.mls).

  • Both Desugarer and Normalization share the same super class DesugaringBase. However, desugaring happens before resolution and normalization happens after resolution, so the helper functions that Desugarer uses should not initialize the implicit arguments when it creates new terms; while the helper functions that Normalization uses should initialize the implicit arguments. Unfortunately they use a same set of helper functions :-( Errors under hkmc2/shared/src/test/mlscript/rp are because of this).

@LPTK
Copy link
Contributor

LPTK commented Jul 15, 2025

Thanks for your WIP! The pattern stuff is completely refactored in #316, so it's not worth dealing with here. Maybe we can just disable the check that makes these tests fail for now, and re-enable it later?

@FlandiaYingman
Copy link
Author

The checks were not there before this PR, so I think we could just wait for Luyu's PR to be merged, and see how we want to change this PR according to that.

@FlandiaYingman FlandiaYingman mentioned this pull request Aug 10, 2025
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