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

Introduce normalizeWithM for monadic normalization #371

Merged
merged 5 commits into from
Nov 6, 2018

Conversation

alexbiehl
Copy link
Contributor

See the discussion in dhall-lang/dhall-lang#133.

@alexbiehl
Copy link
Contributor Author

  • This needs a big comment saying that this might violate assumptions

@Gabriella439
Copy link
Collaborator

Don't forget to export NormalizerM and normalizeWithM from the module

@Gabriella439
Copy link
Collaborator

I'll close this for now since from offline discussions it sounded like you could do this using the existing API by post-processing the abstract syntax tree to monadically normalize uninterpreted free variables

@ocharles
Copy link
Member

ocharles commented Nov 4, 2018

I'd like to re-open this in light of dhallix/nix-derivation#8. There it was determined that the only real option for Dhallix's performance right now is to make derivation a built in, and add an extra step to normalization to avoid blowing up the AST. However, this inherently needs some kind of monadic normalization. I need some way to build up the .drvs that I'm going to write to disk later, to instruct Nix how an output is realised. That could be writer or state, but I can't see a way to do it without adding some effects to normalization.

@Gabriel439 Mind if I resurrect and re-work this PR?

@ocharles ocharles reopened this Nov 4, 2018
@ocharles ocharles self-assigned this Nov 4, 2018
@Gabriella439
Copy link
Collaborator

@ocharles: Yeah, it seems reasonable to proceed with this change

@ocharles ocharles added the wip Work in progress - the author is not yet asking for reviews label Nov 6, 2018
@ocharles ocharles removed the wip Work in progress - the author is not yet asking for reviews label Nov 6, 2018
@ocharles
Copy link
Member

ocharles commented Nov 6, 2018

Ok, I think this is good for review.

@Gabriella439 Gabriella439 merged commit 096c039 into dhall-lang:master Nov 6, 2018
@Gabriella439
Copy link
Collaborator

Thank you, both of you 🙂

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.

3 participants