Update variable resolution #19
Merged
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
This will free up the name which we will need later for kinds of variables stored in environments (as opposed to kinds of references resolved by names).
We have forgot another thing that should have mandatory spans: variable references (see commit e17af9c). All variables have a definition site. Impored variables have a corresponding import declaration. All of them can and should have spans. Make the span non-Optional, remove unwrap()s and expect()s where necessary, update tests to provide some meaningless spans.
We have been using a linear search for variables. While this works well for tests for now, later environments may get really big (e.g., the default imports from (scheme base)). Replace the linear search with hash table lookup keyed by variable name. This is also a good place to introduce more variability in variable kinds. Now we store variables in EnvironmentVariable struct (with their definition site which may get useful later). The variables are distinguished by their kind. Currently there is only one variable kind: runtime variables. Later we will add macro expanders here.
We now store the definition spans of the variables in environments, but they are not returned back when the variables are resolved. Return them. For unresolved variables we have to invent some span. The caller should disregard the returned definition span. I don't like this so it will be encoded into the type system in the next commit.
This reference kind kinda broken the type safety of the references. It could be returned by environments as a result of variable resoltion, but the returned definition span contained bogus value. Instead of this ugly hack we can use a better idiomatic approach with Option. Return Option<VariableReference> from environments, use None for unresolved variables. Assignment looks kinda ugly with that, but that's because we want to produce diagnostics nicely ordered. Maybe I will find a better way...
|
Ugh... well... Travis has failed to launch kcov which fails the build with the following errors: This seems to be a known issue, related to the fact that kcov needs to use the personality(2) system call, which seems to be disallowed by Travis. Not sure why Travis allowed it before or why it is disallowed now. Maybe it's some paranoid overreaction to the recent Meltdown/Spectre vulnerabilities. |
|
This seems to be a issue with Travis infrastructure, so I'm going to merge this PR into the staging branch. Hopefully the issue will be resolved by the time we merge the staging branch into master. |
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.
This is some preparatory refactoring of the variable resolution code so that it is able to support syntactic variables.