-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement support for "name matching processors". #50
Conversation
I think I get it. Although I hadn't heard of (or used)
EDIT: no, it can't. my bad. |
src/lib.rs
Outdated
} | ||
|
||
/// Add a name matching processor: this takes a [HashMap] of `(key, value)` pairs and must | ||
/// return `true` if this is a valid set of pairs or false otherwise. Name matching processors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the key, value
pairs here though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I think I may have cracked it. This function receives the output of the previously applied matchers as a hashmap (they keys are the names, the values are the text the was matched?) and you have the opportunity to inspect it and decide if it's OK for you.
If I understood correctly, what wasn't obvious to me:
- this requires normal name matchers to be used first, the output of which is piped into the input of
name matching processor
. - the name
name matching processor
lead me to think you could "process" (i.e. mutate) something somewhere, but really you just inspect stuff immutably and give a yes/no decision. Wouldname matcher validator
be a better name?
(is the validator called once for each matcher, or with the union of all matchers?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function receives the output of the previously applied matchers as a hashmap (they keys are the names, the values are the text the was matched?) and you have the opportunity to inspect it and decide if it's OK for you.
Correct!
this requires normal name matchers to be used first, the output of which is piped into the input of name matching processor.
Yes. I should have added a check for the "name matching processor requires a name matcher". Fixed in 4b8ad4f.
Would name matcher validator be a better name?
That is less bad. Fixed in 03b3d27.
[I think all of the names in fm could do with a rethink. "Name and pattern" are easily confused, for example.]
is the validator called once for each matcher, or with the union of all matchers
Backtracking makes it pretty much impossible to give a simple bound here, unfortunately.
In combination with ykjit/yk#1311 this seems like the right change to allows to match register names in the assembly output. |
I think this is good to go? Please squash if so. |
03b3d27
to
c4ec81c
Compare
Squashed. |
c4ec81c
to
68d87dd
Compare
I'm an idiot. Force pushed a |
This is an unwieldy name for a complicated-sounding feature that's actually rather simple. In essence, this generalises the previous support we had for "distinct name matching", allowing the user to determine what set of name matching pairs should be considered a successful match or not. For example, distinct name matching is just: ``` .name_matching_validator(|names| { names.values().collect::<HashSet<_>>().len() == names.len() }) ``` Of course, there are other uses this can be put towards! Because I'm a nice person, this commit still supports the `distinct_name_matching` function, though it is deprecated. There is a -- very unlikely -- sequence you could call which would mean that if you turn distinct name matching on and then off, it won't actually turn off: there's only so much I can do.
68d87dd
to
fb6250e
Compare
Hmph, I didn't check debug mode formatting properly. Force pushed a sort-of-formatting fix for that. Hopefully we're now good to go. |
This is an unwieldy name for a complicated-sounding feature that's actually rather simple. In essence, this generalises the previous support we had for "distinct name matching", allowing the user to determine what set of name matching pairs should be considered a successful match or not.
For example, distinct name matching is just:
Of course, there are other uses this can be put towards!
Because I'm a nice person, this commit still supports the
distinct_name_matching
function, though it is deprecated. There is a -- very unlikely -- sequence you could call which would mean that if you turn distinct name matching on and then off, it won't actually turn off: there's only so much I can do.