Skip to content

Fix marshalling of unions (including enums) in and out of Dhall #936

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 5, 2019
Merged

Fix marshalling of unions (including enums) in and out of Dhall #936

merged 3 commits into from
May 5, 2019

Conversation

Gabriella439
Copy link
Collaborator

Fixes #930

@quasicomputational
Copy link
Collaborator

Some cases that I think we'll want tests for:

  • Haskell: data Foo = A () | B, using genericAuto to extract. Dhall: <A : {} | B>.A - shouldn't work as a union constructor, I think? And it shouldn't typecheck against <A | B>.

  • The above, but testing as many branches of the generic machinery as we can.

  • union $ constructor "A" unit likewise shouldn't typecheck against <A>, and <A : {}>.A should have a function type, not a union constructor type.

@Gabriella439
Copy link
Collaborator Author

@quasicomputational: Note that < A : {} | B>.A is a valid union constructor, which has type {} → < A : {} | B >. However, I can still add the tests you suggested

@Gabriella439
Add more tests around constructors storing unit
05b7050 
... as requested by @quasicomputational

This exercises both the generic-deriving machinery, which generates
a record of type `{ _1 : {} }` and the `union`/`inputUnion` machinery, which
converts it to an empty alternative

Note that really constructors with one anonymous field should probably
strip the outer `{ _1 : ... }` record, but that is an orthogonal change
which I will postpone until after this upcoming bug-fix release.  However,
if we were to strip them like that then I would expect the generic-deriving
machinery to also generate an empty alternative, just as the
`union`/`inputUnion` machinery.
@Gabriella439
Copy link
Collaborator Author

@quasicomputational: Also, note that I've decided to make union/inputUnion behave the same as the generic-deriving machinery for now (i.e. treating an empty record as an empty alternative)

@Gabriella439 Gabriella439 merged commit a929d4e into master May 5, 2019
@Gabriella439 Gabriella439 deleted the gabriel/fix_union_marshalling branch May 5, 2019 05:29
@quasicomputational quasicomputational mentioned this pull request May 8, 2019
Merged
Gabriella439 added a commit that referenced this pull request May 11, 2019
@Gabriella439
Mention #936
c2b10fb 
... as caught by @quasicomputational
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Can't load a partial merge application as a Haskell function
2 participants
@Gabriella439 @quasicomputational