Skip to content
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

Get variable and function references from expression #61

Merged
merged 3 commits into from
Jul 9, 2024

Conversation

clarkmcc
Copy link
Owner

@clarkmcc clarkmcc commented Jul 8, 2024

Closes #60

@kameko
Copy link

kameko commented Jul 9, 2024

One thing I would note in the doc comments in ast.rs is that the variables and functions methods in ExpressionReferences clone their internal HashSets, for anyone being memory conscious. This is a practice I picked up in my own code as a courtesy. I wouldn't bother with making it non-clone for now since that can be resolved in a future PR if it's absolutely necessary.

@clarkmcc
Copy link
Owner Author

clarkmcc commented Jul 9, 2024

So I'm assuming you're talking about this

pub fn variables(&self) -> Vec<&str> {
    self.variables.iter().cloned().collect()
}

But that doesn't clone the iterator, it just clones each element which in this case is &&str so we're just dereferencing pointers to &str (i.e. we get an owned T, which for &&str is &str, sorta weird). Because we're borrowing the variables iterator we get references to &str (&&str) which results in &str.

I double-checked my work on this and found that there's actually a .copied() method. In this case it's effectively the same, but I think it more clearly shows what's actually happening so I'll change it to that.

let a: Vec<&&str> = self.variables.iter().collect();
let b: Vec<&str> = self.variables.iter().cloned().collect();
let c: Vec<&str> = self.variables.iter().copied().collect();

@kameko
Copy link

kameko commented Jul 9, 2024

Oh! I didn't even notice, I just took a brief overview of the new API in GitHub (so no type hints). Never mind then!

@clarkmcc clarkmcc merged commit be772a5 into master Jul 9, 2024
1 check passed
@clarkmcc clarkmcc deleted the 60-get-variable-function-references-in-expr branch July 9, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to check which functions/variables a script references
2 participants