Fix equivalence check for Date/Time/TimeZone
#2291
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is fixing a bug where a temporal literal fails to type-check when given a type annotation (or more generally, when the type-checker checks it against an expected type). For example, the expression `00:00:00 : Date` would not type-check. The reason why is that `Dhall.Eval.conv` was missing cases for the `Date` / `Time` / `TimeZone` types and their respective literals, which this change fixes. I also fixed the test suite to catch future issues like this. We do have standard tests that check that temporal literals against expected types, but we were running the tests in such a way that we were not exercising the `Dhall.Eval.conv` code path that had this bug. So I updated the test suite so that it now catches this issue and future issues like this. I verified that the updated test suite now fails without this fix and passes with the fix.
|
I also plan to cut a small |
sjakobi
approved these changes
Aug 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is fixing a bug where a temporal literal fails to type-check
when given a type annotation (or more generally, when the type-checker
checks it against an expected type).
For example, the expression
00:00:00 : Datewould not type-check.The reason why is that
Dhall.Eval.convwas missing cases forthe
Date/Time/TimeZonetypes and their respective literals,which this change fixes.
I also fixed the test suite to catch future issues like this. We do
have standard tests that check that temporal literals against
expected types, but we were running the tests in such a way that we
were not exercising the
Dhall.Eval.convcode path that had thisbug.
So I updated the test suite so that it now catches this issue and
future issues like this. I verified that the updated test suite
now fails without this fix and passes with the fix.