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

Move Environment from TypeChecker to Context #606

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rben01
Copy link
Contributor

@rben01 rben01 commented Oct 8, 2024

This allows not cloning function signatures during elaboration (and I haven't checked, but may obviate other clones as well)

… reason about which functions actually mutate the environment, allowing removal of cloned FunctionSignature

(Previously, removing the clone would have resulted in overlapping mutable+immutable borrows)
@rben01 rben01 changed the title Separate env typechecker Move Environment from TypeChecker to Context Oct 8, 2024
@sharkdp
Copy link
Owner

sharkdp commented Oct 9, 2024

Hm, this complicates the code quite a bit. Do we see significant performance improvements?

@rben01
Copy link
Contributor Author

rben01 commented Oct 9, 2024

I am consistently seeing a roughly -0.25% improvement over master, but I am struggling to get statistical significance from cargo bench. There is probably too much noise for such a small improvement to show up.

Also, while I agree that the function signatures are now more complicated, I generally prefer to make clear where mutability is possible. If the environment really won't be mutated, then I think it makes sense to separate its mutability from the type-checker’s.

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.

2 participants