Lexical lookup from quasis, part Ⅰ #474
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
added 8 commits
February 19, 2019 04:22
These, incidentally, are the ones that require hygiene. Previously we provided that using the `identifier.frame` mechanism, but I'm about to remove that on this branch and put in another one. Before merge, these tests need to work again using the new mechanism. But for now, switching them off like this so that the rest of the test suite stays green.
The former is the general form, when we have a name in code that shouldn't ever do any kind of value lookup, lexical or otherwise. Common examples include dictionary keys, property names, and all kind of definitions (`my`, `class`, `func`, etc.). The latter is a term in an expression. Basically anything that ought to look up a value at runtime should use this form. This change was harder than it looked. A few corners have been cut, and can hopefully be done better in the long term.
A _direct identifier_ is one where no lexical lookup is needed, because the frame and its lexical pad is already known/held. A quasi that's interpolated turns all of its identifiers direct. This makes some quarantined tests pass and so we bring them out of quarantine. However, there's a big piece missing: interpolation shouldn't turn the quasi's *own* variables direct -- those declared somewhere inside the quasi. Because we're not doing this correctly yet, some other tests also needed to move out into quarantine.
This is the wrong fix; it concludes that a variable was declared in the quasi iff lookup for it frame fails. The problem is that lookup might succeed (and find a variable outside of the quasi) but the variable might still have been declared inside the quasi. There's a better fix; it's probably a bit more complicated, and must involve actually declaring those variables inside the quasi block, including in nested blocks. I think the `interpolate` sub will end up looking a lot more like the `check` sub. Because this is the wrong fix, some things are still left in quarantine. However, quite a few things moved back, too.
Adding these from #410; if nothing else, this one is an interesting regression test.
This is the right fix hinted at a couple of commits ago. It's probably not complete, but it does pass all the tests. This brings another test out of quarantine.
I have a feeling this method will need its own mechanism once we get to actually remembering which block variables outside of a quasi come from. For now, this works. This empties out the quarantined tests file, and so we pass all the tests we used to, and then some.
This was referenced Feb 19, 2019
Merged
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 (the first half of) the implementation outlined in issue #410. It's about what happens when a quasi is interpolated.
Part Ⅱ (coming soon to a PR list near you) will be about what happens when such an interpolated quasi is expanded into the mainline. I'm choosing to divide them into two parts because (a) they naturally do that, and (b) this PR is already both at parity with
master, test-wise, and a big improvement.A quick overview of the changes in this PR:
A separation between identifiers which are just names (
Q.Identifier) and identifiers which are used to look up a value (Q.Term.Identifier). This cleared up a number of confusions, not least thatQ.Identifierused to inherit fromQ.Termwithout (always) being a term and without havingTermin the name.Introducing a further sub-type of
Q.Term.Identifier, namelyQ.Term.Identifier.Direct. The former does "regular" lexical lookup; the latter does a "direct" lookup, which means it already knows the frame to go to.As a quasi is evaluated/interpolated, all the
Q.Term.Identifiers we find get turned into.Directones. That way, even as the code gets transported out of the scope where it was created, the right variables will be found.An exception is made for variables that were declared inside the quasi block itself (whether directly inside or nested in blocks inside). These variables shouldn't be made direct, because the whole quasi is a unit and they will never be separated from their declarations.
Besides this, a number of smaller cleanups were made.
.framewas removed completely, both from identifiers and fromQ.Block(‽). This PR removes a ridiculous amount of cruft from failed attempts at hygiene over the years, and replaces them with The Right Approach.I'm super-happy about this. In order to demonstrate how big of a deal this PR is, I will now work on and put up PRs for
infix:<ff>(#207),swap(#218), andpostfix:<++>(#122), all of which should now be possible. (For all of these, we'll have to settle for early versions susceptible to Single Evaluation Rule breakage. Still, better than nothing!)Lastly, I'm working on a blog post about all this; one that I sort of promised in #410.
Closes #387.