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

Updated type inference to not re-type-check the bodies and types of globals #1416

Merged
merged 2 commits into from
Aug 21, 2021

Conversation

eddywestbrook
Copy link
Contributor

The previous implementation of typeInfer for TermF Term performed a mapM typeInferComplete t on terms that were not lambdas or pis. The issue here is that this typeInferComplete then traverses every Term that is a subterm of t, including the bodies of Constant definitions and the types of primitives and constants. This change fixes that issue.

This change also fixes a similar issue in the definition of globalOpenTerm, which now computes the type of a global using scTypeOfGlobal rather than calling into the type inference engine for its type.

…s and primitives when it encounters them; also updated globalOpenTerm to not re-type-check the bodies and types of globals when it uses them
Copy link
Contributor

@brianhuffman brianhuffman 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 great, and it looks like it has a real performance benefit when loading cryptol modules. Thanks!

@eddywestbrook eddywestbrook added the PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run label Aug 20, 2021
@brianhuffman brianhuffman force-pushed the bugfix-global-infer-performance branch from 6e872ac to 0aacaf9 Compare August 20, 2021 22:15
@brianhuffman brianhuffman merged commit 8694e58 into master Aug 21, 2021
@brianhuffman brianhuffman deleted the bugfix-global-infer-performance branch August 21, 2021 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: ready to merge Magic flag for pull requests to ask Mergify to merge given an approval and a successful CI run
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants