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

Rework type checker using "propagate types inward" trick #1283

Merged
merged 22 commits into from
May 28, 2023

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented May 27, 2023

The basic idea is that we try as hard as possible to stay in "checking mode", where we have an "expected type" from the context and "push it down" through a term. We also try hard to avoid generating fresh unification type variables unless absolutely necessary. That way, when we come to something that doesn't match the expected type, we can immediately flag a specific error about that part of the code, rather than later having a generic unification error.

This is mostly just refactoring, but it already improves the type error messages somewhat:

  • Instead of saying something like "can't unify int and text", it might say something like "expected type int, but got text" with a reference to a specific source code location.
  • It's probably possible to still get it to sometimes fail with a "can't unify" type error, but with only this technique I don't think we can ever 100% get rid of that.
  • Type errors now report a specific line and column number, not just a line number like they used to.

There is still a lot more I want to do to improve the type error messages but this is a start.

Closes #99.

-- | Add a source location to a type error and re-throw it.
addLocToTypeErr :: Syntax' ty -> TypeErr -> TC a
addLocToTypeErr s te = case te of
UnifyErr NoLoc a b -> throwError $ UnifyErr (s ^. sLoc) a b
Copy link
Member

Choose a reason for hiding this comment

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

can throwError be moved outside of the case expression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good call, but I actually want to completely rewrite addLocToTypeErr (it should actually have a type like SrcLoc -> TC a -> TC a, and maybe a different name to go with it; it should handle all the different kinds of type errors, to facilitate which we should factor the SrcLoc out so it's not duplicated in every constructor, etc.) so I'd prefer to just wait and do all the refactoring at once.

Co-authored-by: Karl Ostmo <kostmo@gmail.com>
@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label May 28, 2023
@mergify mergify bot merged commit b2747a6 into main May 28, 2023
@mergify mergify bot deleted the feature/propagate-types-inward branch May 28, 2023 02:28
mergify bot pushed a commit that referenced this pull request Jun 3, 2023
Improvements to type inference and type error messages.  Includes:
- More informative error when record literal fields don't match the expected set of fields
- More informative type mismatch error which includes the term where the mismatch occurred.
- Reinstates a case for lambdas with explicit type annotations in inference mode (which was removed in #1283), which helps preserve better error messages in some cases.
@byorgey byorgey mentioned this pull request Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redo type checker using "propagate types inward" trick
2 participants