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

Add textual information for Record and Union diff #1851

Merged
merged 7 commits into from
Jun 17, 2020

Conversation

TristanCacqueray
Copy link
Collaborator

This change improves error messages when argument types are wrong.

Fixes #1846

This change improves error messages when argument types are wrong.

Fixes dhall-lang#1846
@TristanCacqueray
Copy link
Collaborator Author

Example:

$ dhall <<< '<a | b>.a : { a : Bool }'

Use "dhall --explain" for detailed errors

Error: Expression doesn't match annotation

- { … : … } (a record)
+ < … : … > (an union)

@TristanCacqueray
Copy link
Collaborator Author

New examples:

$ dhall <<< '<a | b>.a : { a : Bool }'
Error: Expression doesn't match annotation

- { … : … } (a record type)
+ < … : … > (a union type)

$ dhall <<< '<a | b>.a : { a = Bool }'
Error: Expression doesn't match annotation

- { … = … } (a record)
+ < … : … > (a union type)

$ dhall <<< '{ a : Bool } : { a = Bool }'
Error: Expression doesn't match annotation

- { … = … } (a record)
+ Type

Copy link
Collaborator

@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 all you need to do to fix the build failure is to update the doctest in src/Dhall/Deriving.hs to reflect the new error message

@@ -576,6 +578,7 @@ skeleton (Union {}) =
<> ignore
<> " "
<> rangle
<> " (a union type)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "a union"? It's talking about an instance of a union type, I think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timbertson: No, this is a union type. A union value is of the form UnionType.Alternative x

@timbertson
Copy link
Collaborator

Thanks! Don't suppose you could add "(a function)" to the Lam skeleton too? I think that's rendered as … → … which is a bit less cryptic but could still benefit from some words imo.

@TristanCacqueray
Copy link
Collaborator Author

@timbertson last commit adds "(a function)" message to Lam, though that doesn't work well:

$ dhall <<< '(\(a : Bool) -> a) : Bool'
Error: Expression doesn't match annotation

- Bool
+ … → …

$ dhall <<< 'Bool : (\(a : Bool) -> a)'
Error: Expression doesn't match annotation

- λ(… : …) → … (a function)
+ Type

And that wouldn't work for return type:

$ dhall <<< '(\(a : Bool) -> 42) : forall (a : Bool) -> Bool'
Error: Expression doesn't match annotation

  …
→ - Bool
  + Natural

I guess "(a function)" needs to be added to Pi instead, but I'm not entirely sure why it shouldn't be "(a function type)" :)

@sjakobi
Copy link
Collaborator

sjakobi commented Jun 15, 2020

(a function) seems fine to me for Lam. And for consistency, Pi should get (a function type).

Thanks BTW – I think this will make some errors much easier to understand! :)

@sjakobi
Copy link
Collaborator

sjakobi commented Jun 15, 2020

BTW, Hydra OOM'd again (error 137), with GHC 8.4.3. This has been happening repeatedly in the last few days, possibly due to more contributions during ZuriHac.

I'm wondering though whether it might be time to upgrade to a machine with more RAM. If costs are an issue, maybe it could be financed from the donations?

@Gabriella439
Copy link
Collaborator

@sjakobi: Cost would be an issue 🙂. I currently pay $40 / month for a Linode with 8 GB of RAM. The next tier with 16 GB would be $80 / month, which would consume a significant fraction of our donation budget.

I would personally wait to see if it's still a problem now that Zurihac's over

@sjakobi
Copy link
Collaborator

sjakobi commented Jun 15, 2020

Sure. Of course we could also look at cheaper providers, as for example static-haskell-nix did, but it's all work that isn't strictly necessary when the existing server can mostly shoulder the load.

@sjakobi
Copy link
Collaborator

sjakobi commented Jun 16, 2020

@Gabriel439 I believe the OOM'ing Hydra job needs to be restarted: https://hydra.dhall-lang.org/build/61554/nixlog/2

@mergify mergify bot merged commit 4aabe1c into dhall-lang:master Jun 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Papercut: incorrect kind errors are somewhat cryptic
4 participants