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

WIP: Type error display improvements #49

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

david-christiansen
Copy link
Collaborator

@david-christiansen david-christiansen commented Apr 24, 2020

While writing pmatch, it was often difficult to figure out what Klister meant when giving type errors. Many of them were due to #48, it turned out, but that was made hard to see. This PR doesn't make it good, but it is at least non-terrible now:

  1. Type errors are aggregated within a compilation unit (that is, a dyamic extent delimited by a completely) and reported as a group.
  2. Unification failures display the complete types that failed to unify, as well as their specific sub-components that failed, when these differ.

This is not quite ready for review/merge yet, due to one downside: unification failures do not prevent the elaboration of a term that can't type check, but they should. As things stand now, ill-typed macro code could be executed (though not ill-typed phase 0 code). I need to ponder the right design for fixing this - probably, the stuff in Expander.TC should run in a Validation applicative functor with a bit of local state to track the union-find structure, rather than an Error monad with the global expander state. The interface between Expand and TC can then check how many failures occurred and spit out an explicit representation of the error into the Core that results. Or something like that.

Also, the work is done on top of #47, because I don't expect to finish this before that's been merged, and because I have some little local tests that use pmatch in examples.

Type errors are improved by doing the following:

 1. Normalize types in them as far as possible, so it's not just a TyF
 around METAs in the error message

 2. When the unification error occurs in a nested call to unify, call
 that out but also save the original types. Show the inner fail in a
 "Specifically" block as GHC does.
This makes the code easier to understand and improves GHC warnings.
Type errors are now collected within the extent of a call to
completely, and aggregated for later display.
@david-christiansen david-christiansen marked this pull request as draft April 24, 2020 20:29
@david-christiansen
Copy link
Collaborator Author

Having thought about this for a while, I think I have an idea of how to proceed. First of, we must recognize that macros are the source of all compile-time execution, and free variables in macros always refer to either imports or definitions (rather than e.g. let bindings). We do not allow modules to load others with type errors. This means that it is sufficient to ensure that no macro that transitively refers to a definition with a type error is ever executed.

I propose to do that by maintaining a mapping from defined names to their type errors and their dependencies' type errors. When we add a macro to the transformer environment (that is, the handler for TaskAwaitingMacro, we first check whether it has type errors or the free variables are definitions that transitively have type errors. If so, we replace the macro with a dummy macro action that explains why it can't run, pointing at the type errors that must be fixed.

This means that modules with type errors at phase 1 or higher may not report all type errors, because a macro failing to run may interrupt expansion of the rest of the module. But I think it's a reasonably simple compromise that's much better than only ever showing one type error at a time!

Base automatically changed from master to main January 12, 2021 23:13
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.

1 participant