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

Codegen/Rust: allow renaming in QL #18094

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Codegen/Rust: allow renaming in QL #18094

merged 3 commits into from
Nov 25, 2024

Conversation

redsun82
Copy link
Contributor

This adds a ql.name codegen pragma to change the name of a property on the QL side. This is useful to give more meaningful names than what we get from the generated rust AST.

This is used as an example for MatchExpr.getExpr, renamed to MatchExpr.getMatchedExpr.

This adds a `ql.name` codegen pragma to change the name of a property on
the QL side. This is useful to give more meaningful names than what we
get from the generated rust AST.
@redsun82 redsun82 requested a review from a team as a code owner November 25, 2024 16:05
@github-actions github-actions bot added the Rust Pull requests that update Rust code label Nov 25, 2024
@redsun82 redsun82 marked this pull request as draft November 25, 2024 16:05
geoffw0
geoffw0 previously approved these changes Nov 25, 2024
Copy link
Contributor

@geoffw0 geoffw0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM.

Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe Rust uses the terminology scrutinee, so perhaps getScrutinee instead?

@redsun82
Copy link
Contributor Author

I believe Rust uses the terminology scrutinee, so perhaps getScrutinee instead?

Ah, that is what @paldepind suggested in his request for this, I didn't know it was an official rust terminology. Matched expression seems more immediate to me, but I won't argue with established terminology! Will do

@redsun82 redsun82 marked this pull request as ready for review November 25, 2024 16:24
@paldepind
Copy link
Contributor

Thanks a lot @redsun82. This is great and I think this (and doing the same for other getExpr predicates) can make things quite a bit more readable and solve the problem I mentioned here #17918 (comment) (where the getExpr in the CFG AST layer ends up getting shadowed by other getExpr predicates).

@redsun82 redsun82 merged commit 74aa47a into main Nov 25, 2024
22 of 23 checks passed
@redsun82 redsun82 deleted the redsun82/rust-rename branch November 25, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants