Improve handling of top-level names with new ResolveBehaviour system#270
Improve handling of top-level names with new ResolveBehaviour system#270IsaacWoods merged 2 commits intorust-osdev:mainfrom
ResolveBehaviour system#270Conversation
|
|
||
| pub fn new_with(op: Opcode, arguments: Vec<Argument>, more: usize) -> OpInFlight { | ||
| OpInFlight { op, expected_arguments: arguments.len() + more, arguments } | ||
| fn resolve_behaviour(&self) -> ResolveBehaviour { |
There was a problem hiding this comment.
Purely for my own benefit, I'd like to check my understanding here... Is this function effectively "get the resolve behaviour for the next argument to be pushed into this op"?
There was a problem hiding this comment.
Yes exactly - I've added a comment to make this clearer.
|
I'm not sure whether or not you're looking for a second pair of eyes, but I thought I'd take a look just to help my understanding of your parser. I haven't checked whether all the ResolveBehaviours are correct but the concept of what you've done makes sense to me, and I couldn't think of any real suggestions. (I'd love to get rid of the numeric Plus I've done a quick test and it seems to work. Thanks again for all your work on this crate! Kudos on the British spellings BTW 🫡 |
|
Second pair of eyes is always very welcome - easy to make silly mistakes. I tend to leave PRs for a day or two and come back for a re-review anyways. I was very tempted to get rid of
Haha, yes I seem to have settled on a half-way house where I use |
|
🚀 Published as |
2 participants
This PR aims to improve handling of names at the operand level by encoding the expected resolution behaviour for each argument to an operation that is in-flight. This replaces the ad-hoc behaviour we had before, which was not sufficient to handle
SuperNameoperands correctly.This should be a definitive fix for #262, replacing the previous solution in db2ac63 which only handled
DefStore.Fixes #262