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

Fix false interpreter #7369

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Fix false interpreter #7369

wants to merge 8 commits into from

Conversation

bhansconnect
Copy link
Contributor

Gets the false interpreter upgraded to purity inference, cleaned up in general, and running as a test again.

smores56
smores56 previously approved these changes Dec 14, 2024
Copy link
Collaborator

@smores56 smores56 left a comment

Choose a reason for hiding this comment

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

So much cleaner with purity inference. Good change!

@@ -11,132 +11,124 @@ import Variable exposing [Variable]
# It has some extra constraints:
# 1) The input files are considered too large to just read in at once. Instead it is read via buffer or line.
# 2) The output is also considered too large to generate in memory. It must be printed as we go via buffer or line.

# All of the notes below this line are hopefully outdated with purity inference.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nitpick: maybe prefix this with a TODO so we remember to remove the outdated notes if this successfully fixes the issue they point out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Removed. I think it is all resolved.

Copy link
Contributor Author

@bhansconnect bhansconnect Dec 14, 2024

Choose a reason for hiding this comment

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

Also, false feels surprisingly snappy for what it is.

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