Refactor TopEnvFrag #861
Open
Refactor TopEnvFrag #861
Conversation
This file contains 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
TopEnvFrag was very confusing, because even though it used to carry a ModuleEnv, many elements of that ModuleEnv (source map, synthesis candidates, ...) would be expected to not include any imported values! I lost some time thanks to this unstated invariant, so I figured it's a good time to clean it up! With this change `TopEnvFrag` directly mimics the structure of `Env`, and includes top emissions separately from module-level emissions. And module-level emissions are no longer represented by a `ModuleEnv`, but by a `ModuleEmissions` ADT that explicitly states the relevant invariants.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
TopEnvFrag was very confusing, because even though it used to carry a
ModuleEnv, many elements of that ModuleEnv (source map, synthesis candidates, ...)
would be expected to not include any imported values! I lost some time
thanks to this unstated invariant, so I figured it's a good time to
clean it up!
With this change
TopEnvFragdirectly mimics the structure ofEnv,and includes top emissions separately from module-level emissions. And
module-level emissions are no longer represented by a
ModuleEnv, butby a
ModuleEmissionsADT that explicitly states the relevantinvariants.