Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #227 +/- ##
==========================================
+ Coverage 78.23% 78.70% +0.46%
==========================================
Files 63 63
Lines 3690 3789 +99
==========================================
+ Hits 2887 2982 +95
- Misses 803 807 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
dd75036 to
d078a3c
Compare
863b0d6 to
3d7fd5d
Compare
3d7fd5d to
dbbeda7
Compare
This file contains hidden or 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
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.
Related Links
Description and Rationale
applyis now a fully-supported SKEL functionmake debug-testrhs_to_valuefunction in the event that the first component isn't a variableHow
Checking defined variables in AST generation
It turns out we were already tracking a defined_vars set in the ast code, so it was a "relatively" simple matter of plumbing that through to more places. It's a little subtle/tricky, because in the left-hand-side of a resource test, you can only reference the variable that's defined in that test, but in the right-hand side you can reference variables that were defined in previous tests. In other words, this is invalid:
But this is valid:
We also got rid of the
var_prefixed_resource_pathrule in the grammar, because it turns out a) it over-complicated the grammar, and b) it disallowed some valid expressions.Apply SKEL command
This was more-or-less straightforward, we just needed to ensure that all the different variants in the assignment worked correctly (e.g.,
path = val,$x.path = val,path = $y, and$x.path = $y).We created a reify_pointers function which takes an abstract pointer and the variable context, and converts that to a list of concrete pointers that match the object with all the variables filled in. This actually simplifies the logic for the
removecommand as well, and fixes a couple corner cases I didn't think about before (specifically, if the variable substitution doesn't work because we didn't reference a particular variable in the abstract pointer, that would have silently gone through before; I don't think it actually would do anything but still probably isn't ideal).Test Steps
Other notes
When I was writing this before, I forgot about the
klabelmacro, which is super-handy. Would love to get some of this stuff extracted out into a shared library at some point.