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

Encode doubles as Double, do bounds checks, and add support for NaN and Infinity values #667

Merged
merged 22 commits into from
Nov 20, 2018

Conversation

drvirgilio
Copy link
Contributor

See discussion in dhall-lang issue #252. I'm not sure exactly on where the parser for "[-]Infinity" should go so I kind of guessed.

Copy link
Collaborator

@Gabriella439 Gabriella439 left a comment

Choose a reason for hiding this comment

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

Looks great! I'll merge this once the upstream change to the standard is merged

dhall/src/Dhall.hs Outdated Show resolved Hide resolved
@drvirgilio
Copy link
Contributor Author

I merged the changes to the standard so I think this is ready to be merged.

Copy link
Member

@f-f f-f left a comment

Choose a reason for hiding this comment

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

@drvirgilio LGTM 👍
Could we add some tests that exercise the new keywords and conversions?

@Gabriella439
Copy link
Collaborator

@f-f: I'll go ahead and merge this now and add tests later, because I need to sync this repository with the dhall-lang test suite first. The other reason why is that I would like to add the tests to dhall-lang first and then mirror them to this repository rather than the other way around

@Gabriella439
Copy link
Collaborator

Actually, there are test failures on this branch that are being hidden by a CI misconfiguration on my part. I'm going to fix that first in a separate pull request and then fix the tests on this branch

@Gabriella439
Copy link
Collaborator

I have a pull request up against this branch with fixes to the test suite: drvirgilio#1

@drvirgilio
Copy link
Contributor Author

Finally got around to making a few changes:

  1. Added tests which check a few boundary conditions such as the point where Integer/toDouble rounds up to Infinity.
  2. Found a workaround for fromInteger's weird rounding behavior by using read . show.
  3. Made the changes suggested by @Gabriel439

@Gabriella439 Gabriella439 merged commit adf94a6 into dhall-lang:master Nov 20, 2018
@Gabriella439
Copy link
Collaborator

@drvirgilio: Thanks for doing this! 🙂

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.

None yet

4 participants