Recursive type checker#1289
Open
cormacrelf wants to merge 10 commits into
Open
Conversation
We will later use this to disallow duplicate type annotations. That's why we include error_span, etc.
For use in BindingsCollect.
Previously, we gathered and solved bindings for every top-level def, and not for a module's root bindings. This was a deliberate limitation for typechecker performance reasons, introduced in D48752028/D48752027 back in 2023. This completes the work described in those diffs, which said ideally: 1. typecheck top-level assignments 2. then evaluate defs This does one better and typechecks even nested defs individually. This achieves the goal of reducing the iterations needed in solve_bindings by passing the smallest possible quantity of bindings to solve_bindings at any one time.
This makes the prelude type-check again after the recursive type checker was introduced. Errors were e.g 'RunInfo | None' not finding binop |.
meta-cla
Bot
added
the
CLA Signed
Contributor
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D100318302. (Because this pull request was imported automatically, there will not be any future comments.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Just this part of #1148. It is described most fully in the doc comment in typecheck.rs. As a reminder, the big impact is that we now typecheck top level assignments and function calls.
OSS buck is not actually building right now (#1280). But with some dumb fixes to those issues applied on top, this builds.
I added one extra change 'Fix TyUser not forwarding bin_op' so that the prelude typechecks. i.e.
buck2 starlark typecheck prelude/**/*.bzl. There were only failures because these things always appeared in top-level bindings e.g.Blah = provider(fields = ... RunInfo | None, ... ). Under real world usage I don't know, run it on your systems and see what shakes out.Attn @Wilfred who wanted this separate in 1148.