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

Allow all annotations to take full expressions #1230

Merged
merged 2 commits into from
Oct 21, 2021

Conversation

MonoidMusician
Copy link
Collaborator

Instead of just application expressions, since some annotations were allowed to take full expressions, it makes sense to allow them all to do so.

Otherwise there are weird corner cases like
merge { x = λ(y : Bool) → y } < x >.x : (∀(y : Bool) → Bool)
parses as a merge-with-annotation while
merge { x = λ(y : Bool) → y } < x >.x : ∀(y : Bool) → Bool
parses as a merge-without-annotation plus annotation, just like
(merge { x = λ(y : Bool) → y } < x >.x) : ∀(y : Bool) → Bool
(from https://github.com/dhall-lang/dhall-lang/blob/1907a1d1a6dff9ff8638547f7bc49d6b5135bcdf/tests/type-inference/success/unit/MergeOneWithAnnotation1A.dhall)

Now the second will parse like the first, instead of like the last.

Of course we should discuss whether this is a breaking change, but I think the actual user-facing effects will be minimal, since the difference in annotations are normalized away.

This came up in fixing my pretty printer – I don't think there's a good way for me to disambiguate the old way correctly within my current system of precedence.

I also removed trailing spaces in a separate commit and noticed that the file uses CRLF line endings which is very weird. Happy to revert that commit if you want …

Instead of just application expressions, since some annotations
were allowed to take full expressions, it makes sense to allow
them all to do so.

Otherwise there are weird corner cases like
  merge { x = λ(y : Bool) → y } < x >.x : (∀(y : Bool) → Bool)
parses as a merge-with-annotation while
  merge { x = λ(y : Bool) → y } < x >.x : ∀(y : Bool) → Bool
parses as a merge-without-annotation plus annotation, just like
  (merge { x = λ(y : Bool) → y } < x >.x) : ∀(y : Bool) → Bool

Now the first two will parse the same.
Copy link
Contributor

@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.

I think it's a totally fine breaking change that's unlikely to affect many people in practice

@Gabriella439 Gabriella439 merged commit 61f291e into dhall-lang:master Oct 21, 2021
@Gabriella439
Copy link
Contributor

Sorry, I totally forgot to merge this! 😅 Thank you for doing this

Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Oct 21, 2021
Gabriella439 added a commit to dhall-lang/dhall-haskell that referenced this pull request Oct 23, 2021
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

2 participants