Fix isNormalized for field selections #1210
Merged
Conversation
This file contains hidden or 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
In expressions like
({ x : Optional/fold } // {=}).x
isNormalized didn't check if the expression being selected from
was normalized.
Fixes #1209.
3517a95 to
4fe4a0b
Compare
Gabriella439
approved these changes
Aug 7, 2019
| Combine x@(RecordLit m) y -> loop x && loop y && Dhall.Map.member k m | ||
| Combine x (RecordLit m) -> loop x && Dhall.Map.toList (fmap loop m) == [(k, True)] | ||
| Prefer x@(RecordLit m) y -> loop x && loop y && Dhall.Map.member k m | ||
| Combine (RecordLit m) _ -> Dhall.Map.member k m && loop r |
There was a problem hiding this comment.
Perhaps move the loop r check above so that does not have to be repeated across the branches, like this:
Field r k -> loop r && case r of
RecordLit _ -> False
…There was a problem hiding this comment.
I actually did that in a previous version of this fix, but I just couldn't handle the inefficiency! ;)
If you don't mind too much I'd slightly prefer to keep the fix as it is now.
There was a problem hiding this comment.
I don't mind! It was just a suggestion 🙂
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.
In expressions like
isNormalizeddidn't check if the expression being selected fromwas normalized.
Fixes #1209.