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

Proper type instantiation discipline #1473

Merged
merged 7 commits into from Jul 25, 2023
Merged

Proper type instantiation discipline #1473

merged 7 commits into from Jul 25, 2023

Conversation

yannham
Copy link
Member

@yannham yannham commented Jul 24, 2023

Closes #584 #360 #1027

This PR is the last missing piece toward a reasonable bidirectional typechecking algorithm that properly handles higher-rank polymorphic types.

Until now, instantiation of polymorphic types has been sometimes ad-hoc and bizarrely limited. Simple programs such as:

let r : { id : forall a. a -> a } = { id = fun r => r } in
r.id "x"

or

let eval : forall a. (forall b. b -> b) -> a -> a = fun f x => f x in
(eval (fun x => x) 1) : Number

wouldn't typecheck (cf related issues at the top). Variable levels introduced in #1372 were an important preliminary step before we could attack this issue.

This PR is building on #1372 to use a proper instantiation discipline, derived from Complete and Easy Bidirectional Typechecking for Higher-Rank
Polymorphism
(ideally from the specification of the type system in this repository, but it's unfortunately not complete enough yet to be used as a reliable source of truth).

This commit gets rid of the ad-hoc cases of instantiation for Op1 applications, the variable rule and others: those are now all subsumed by the subsumption rule, because they are special cases of a switch from inference mode to checking mode. Now, check and subsumption are the only place where the checked type might be instantiated with rigid type variable (I suspect we might even get rid of the instantiation of the checked type in subsumption, because it's always done by check before anyway, but this has to be verified first), and subsumption is the only rule to instantiate an inferred type with fresh unification variables.

The result is that the previously mentioned cases, plus other examples of higher-rank polymoprhic functions are now typechecking just fine.

Funnily, one test in typecheck/fail was actually well-typed, but didn't typecheck because of the described previous limitations. It's just been transformed to a passing test.

TODO

  • actually enforce that the instantiation is predicative (mark unification variables coming from the instantiation of a polytype and avoid unifying them with non instantiated polytypes). Currently we do naive impredicative instantiation, but it's just because I didn't want to load this PR too much: it's going to be predicative before the next release. Not that the current version isn't unsound, but doing naive impredicative instantiation makes typechecking sensitive to arguments order for such impredicative cases.

@github-actions github-actions bot temporarily deployed to pull request July 24, 2023 09:44 Inactive
@yannham yannham marked this pull request as draft July 24, 2023 10:22
@github-actions github-actions bot temporarily deployed to pull request July 24, 2023 11:22 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 24, 2023 14:05 Inactive
@github-actions github-actions bot temporarily deployed to pull request July 25, 2023 10:20 Inactive
core/src/typecheck/mod.rs Outdated Show resolved Hide resolved
core/src/typecheck/mod.rs Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to pull request July 25, 2023 12:20 Inactive
@yannham yannham marked this pull request as ready for review July 25, 2023 12:21
Copy link
Member

@vkleen vkleen left a comment

Choose a reason for hiding this comment

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

This looks reasonable, with the caveat that I'm not an expert in the current Nickel typechecker 😅

core/tests/integration/main.rs Outdated Show resolved Hide resolved
Comment on lines +2096 to +2098
// It might make sense to accept any type for a value without definition (which would
// act a bit like a function parameter). But for now, we play safe and implement a more
// restrictive rule, which is that a value without a definition has type `Dyn`
Copy link
Member

Choose a reason for hiding this comment

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

Is this a change in behaviour or was this just implicit in the previous typechecking code?

Copy link
Member Author

@yannham yannham Jul 25, 2023

Choose a reason for hiding this comment

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

This is a change in behavior, but I think the previous behavior was accidental, and neither officially supported nor documented. The previous behavior was indeed to let the type of fields without values be unconstrained.

@github-actions github-actions bot temporarily deployed to pull request July 25, 2023 13:25 Inactive
yannham and others added 7 commits July 25, 2023 15:26
This commit is the last missing piece (at least for the time being)
toward a reasonable bidirectional typechecking algorithm that properly
handle higher-rank types.

Instantiation of polymorphic types has been ad-hoc and full of issues.
Simple programs such as:

```nickel
let r : { id : forall a. a -> a } = { id = fun r => r } in
r.id "x"
```

or

```nickel
let eval : forall a. (forall b. b -> b) -> a -> a = fun f x => f x in
(eval (fun x => x) 1) : Number
```

wouldn't typecheck. Previously introduced variable levels were required
before we could change that.

This commit is building on the variable level to use a proper
instantiation discipline from the specification of the Nickel type
system located in this repository, and the original inspiration, [A
Quick Look at Impredicativity]() (although type instantiation is
currently predicative, the general structure of the bidirectional
type system is similar). Doing so, we also move code between `check` and
`infer` to better reflect the specification: `infer` is not
wrapper around check + unification anymore, but it actually implements
infer rules (function application, primop application, variable and annotation).

Instantiation is taken care of when switching from infer mode to check
mode within `subsumption` and at function application.
And fix the error expectation for `TypecheckError::VarLevelMismatch`
that still had the old longer name `VariableLevelMismatch`.
Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
@github-actions github-actions bot temporarily deployed to pull request July 25, 2023 13:38 Inactive
@yannham yannham enabled auto-merge July 25, 2023 13:51
@yannham yannham added this pull request to the merge queue Jul 25, 2023
Merged via the queue into master with commit 0f02aa1 Jul 25, 2023
5 checks passed
@yannham yannham deleted the fix/type-instantiation branch July 25, 2023 14:49
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.

Implement a proper bidirectional algorithm
2 participants