Parsing attribute access#281
Conversation
This has still a critical flaw when parsing attributes of the same name from two different objects. I'm going to commit as-is and then see if Claude will work with this more like human review process. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
This actually goes too far, I think we can relax it. But Claude's first pass being more relaxed had bugs, so let's tighten all the way down then loosen it up one case at a time. I'll tolerate the churn for a bit of precision. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
This isn't strictly a problem with attribute access, but attribute access is how I found it. We now use a new field on the symbol scope to keep pinned names unavailable more robustly across scopes Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Liam Huber <liamhuber@greyhavensolutions.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## parser-qol #281 +/- ##
=============================================
Coverage 100.00% 100.00%
=============================================
Files 39 41 +2
Lines 3105 3286 +181
=============================================
+ Hits 3105 3286 +181 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
My takeaway from this is that the difficult thing with injection is that one needs to be much, much more careful about tracking symbol availability in different scopes. I'm not thrilled with the extra parser complexity here to achieve this. However, here, as a workflow writer myself, I find I really, really want the feature. And here @XzzX confirms that he also expects workflow writers to want the same capability:
So although I have some misgivings around the additional complexity, I'm going to proceed with merging it down into the landing pad under the conclusion that the functionality boost outweighs the complexity pain. |
This PR lets the parsers interpret
.-based attribute access on symbols by injecting get-attribute nodes from the standard library.These work pretty much anywhere with three restrictions:
The first two are super easy to understand. The third can be seen in the example below. The
static.valcall is fine, because it's going to have the same value for the whole loop. Thelooped.valcall works fine in python, but in the graph manager we don't manage the reference the same way, and the connection between thelooped.valand the newloopedgets lost. Trying to bump into this fails at parsing time, clearly, and with a productive suggestionWe can bind it outside and redeclare it inside to make it loop as expected