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

Improved type inference #846

Open
wants to merge 7 commits into
base: development
Choose a base branch
from

Conversation

supercooldave
Copy link

@supercooldave supercooldave commented Jul 4, 2017

This PR relaxes the type checking in the compiler dealing with unknown information (BottomType), which arises for instance in expressions like Nothing where the parameter to Maybe[_] is not provided nor can be inferred from the context. This PR permits such code to compile because the presence of BottomType does not affect the compilation.

Some previously failing tests now pass.

The key test is in regressions/836.enc because this PR builds directly upon the PR that fixed that issue (#841).

Also, fixes #792

Copy link
Contributor

@EliasC EliasC left a comment

Choose a reason for hiding this comment

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

I see the point of this and the changes are straightforward, but I'm wondering if we're optimising for toy programs rather than "real" code? For example, the following expression now compiles (which makes sense)

match Nothing with
  Nothing => 42
end

but at the same time, the following function (still) does not compile and arguably gives a worse error message than before:

fun lookup[k : Eq[k], v](key : k, arr : [(k, v)]) : Maybe[v]
  var elem = Nothing
  for e <- arr do
    if e.0.eq(key) then
      elem = Just(e.1)
    end
  end
  return elem
end

Not enough information to infer the type.
Try adding more type information.
In expression:
Just(e.1)
In expression:
elem = Just(e.1)

The function is fixed by adding type annotations, not where the typechecker is pointing, but where elem is declared. Before this PR, the typechecker would indeed fail at the declaration site.

Does it make sense to require type annotations for, say, variables and functions whose values (bodies) contain a bottom typed value?

@supercooldave
Copy link
Author

@EliasC The situation you refer to involves multiple different statements, as opposed to a single expression. Checking of bottomness doesn't span different statements.
Of the top of my head, I'm not sure how best to handle this.
Perhaps the error message could include some better hints, such as where good places to add the type annotations are.

@@ -54,7 +54,6 @@ import AST.AST as AST
import Data.List
import Data.Maybe
import Text.Printf (printf)
import Debug.Trace
Copy link
Contributor

Choose a reason for hiding this comment

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

We will most likely need this again :)

Copy link
Author

Choose a reason for hiding this comment

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

I thought we were finished with tracing. My bad.

@EliasC
Copy link
Contributor

EliasC commented Jul 13, 2017

What I'm proposing is to disallow val x = Nothing and give the error about type annotations there, but still allow e.g. match Nothing with .... If you assign Nothing to a variable, chances are very high that you are planning to re-assign it, which will lead to an error. We should therefore catch this error as early as possible.

@supercooldave
Copy link
Author

@EliasC Sounds complicated to check and awfully specific. Would [Nothing] be allowed? Or is the assignment of a bottom-typed value disallowed?

@EliasC
Copy link
Contributor

EliasC commented Jul 13, 2017

I'll be more clear:

An assignment where the right-hand side has a type that contains the bottom type should not be allowed without type annotations. Similarly, I think that closure whose body has a type that contains the bottom type should not be allowed without type annotations. All other usages of the bottom type can be allowed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restrictive forward's type
2 participants