Skip to content
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

Improvements and fixes for UCS #157

Merged
merged 58 commits into from
May 22, 2023

Conversation

chengluyu
Copy link
Member

@chengluyu chengluyu commented Mar 12, 2023

Change Logs

  • Add a complete test cases in UCS: untyped lambda calculus.
  • WIP test cases in UCS: simply typed lambda calculus.
  • Support alias clauses (e.g. ... and f(x) is y and ...).
  • No longer report inexhaustiveness when there is a default branch.
  • Fix missing interleaved computations in nested branches (examples added in d93e0d6).
  • Fix unhygienically extracting positional class parameters as described in UCS tracking issue #151.
  • Mark the source of let bindings so no we won't promote let bindings which is only accessible in a case branch to the level of its parent case expression. (Mentioned in this reply of UCS tracking issue #151)
  • Fix empty warnings of duplicated branches as described in UCS tracking issue #151.
    fun testF(x) = if x is
      Foo(a) then a
      Foo(a) then a
    //│ ╔══[WARNING] duplicated branch
    //│ ╙──

Goals

Non-Goals

  • Error messages and location reporting. This PR focuses more on making the UCS feature fully functional and minimizing errors.

@chengluyu
Copy link
Member Author

Virtual Constructors

Two ideas for implementing virtual constructors (e.g. tuples).

  1. Treat Tuple#n as an alias of tuple types in the typer. Then, translate Tuple#n as JavaScript predicates in the JS backend.
  2. Type predicate function isTuple(t, n) in the typer and desugar tuple patterns to terms with reference to the function. Implement isTuple as a polyfill function in the JS backend.

@chengluyu chengluyu changed the base branch from mlscript to new-definition-typing May 4, 2023 09:53
@chengluyu chengluyu marked this pull request as ready for review May 5, 2023 06:59
Copy link
Member Author

@chengluyu chengluyu left a comment

Choose a reason for hiding this comment

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

I have checked all the test files and updated the tracking issue.

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.

Wow, impressive changes! But they still need a little polish.

shared/src/main/scala/mlscript/NewLexer.scala Outdated Show resolved Hide resolved
js/src/main/scala/Main.scala Show resolved Hide resolved
shared/src/main/scala/mlscript/ucs/Clause.scala Outdated Show resolved Hide resolved
shared/src/main/scala/mlscript/ucs/Desugarer.scala Outdated Show resolved Hide resolved
shared/src/main/scala/mlscript/ucs/Desugarer.scala Outdated Show resolved Hide resolved
shared/src/test/diff/nu/BadUCS.mls Show resolved Hide resolved
shared/src/test/diff/nu/repro_EvalNegNeg.mls Outdated Show resolved Hide resolved
shared/src/test/diff/ucs/DirectLines.mls Show resolved Hide resolved
shared/src/test/diff/ucs/Exhaustiveness.mls Show resolved Hide resolved
shared/src/test/diff/tapl/NuSimplyTyped.mls Outdated Show resolved Hide resolved
@chengluyu chengluyu requested a review from LPTK May 12, 2023 18:07
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.

Thanks, otherwise this looks good to me!

I'm a bit uncomfortable with the large code duplication introduced in your examples, but we can probably move these examples to the new compiler diff-tests later, where we'll be able to share some code in library files.

shared/src/test/diff/tapl/SimplyTyped.mls Outdated Show resolved Hide resolved
shared/src/test/diff/nu/BadUCS.mls Show resolved Hide resolved
shared/src/test/diff/parser/NuParser.mls Outdated Show resolved Hide resolved
shared/src/test/diff/ucs/HygienicBindings.mls Outdated Show resolved Hide resolved
@LPTK
Copy link
Contributor

LPTK commented May 17, 2023

Ah, the build fails due to warnings. There were 34 before this PR and 35 now. Could you please try to solve the most obvious warnings, including specifically the one warning you introduced?

@chengluyu
Copy link
Member Author

Sure. I will handle them later.

@LPTK
Copy link
Contributor

LPTK commented May 17, 2023

Thanks! If there are some you're not sure how to deal with, leave them and I'll address them myself.

@chengluyu
Copy link
Member Author

chengluyu commented May 18, 2023

I tried to fix some warnings and non-exhaustive pattern errors. Now there remains 30 of them. It seems to go beyond the scope of what this PR should be responsible for if all of them need to be fixed. For example, the warning about asInstanceOf. So, I'll just do this much for now.

@LPTK
Copy link
Contributor

LPTK commented May 18, 2023

Yes that's fine, we'll deal with the other warnings later. Let me know when I can merge.

@LPTK
Copy link
Contributor

LPTK commented May 22, 2023

@chengluyu can I merge soon? One conversation is still open.

@LPTK LPTK merged commit 8ca4e93 into hkust-taco:new-definition-typing May 22, 2023
1 check failed
@LPTK LPTK deleted the ucs-paper branch May 22, 2023 16:25
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.

UCS tracking issue
2 participants