-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat(compiler): Support pattern matching "or" patterns #1173
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good (matchcomp.re
is definitely cleaner now), but I have a couple of questions before I hit approve.
Does this need formatter support @ospencer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good! We need to switch over to ocaml's Either since we upgraded to 4.12 and I had some other comments.
It looks like the snapshots are outdated so I didn't look at them.
808d9ce
to
7765b9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 Let's GOOOO!
This PR refactors the match compiler to properly support or-patterns. The two major changes are how bindings work and how constant patterns are compiled. Slots for each of the bindings needed in each of the branches are created, and during the branch selection process these slots are filled with the proper values. Constant patterns are now compiled the same way constructors are and should be more efficient now.
This opens the door for a number of optimizations we can write, but I left them out of this PR to keep it relatively simple.
You may notice that a fair number of snapshots are a bit bigger now; we need a few optimizations to remove the cruft, which will come after this PR.
Closes #264