-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Rust: Add ComparisonOperation library. #19535
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
Conversation
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.
Pull Request Overview
Adds a new ComparisonOperation library to model comparison, equality, and relational operations in Rust QL, updates imports and cleans up legacy operand predicates, and extends library-tests to verify the new classes and operand tags.
- Introduced
ComparisonOperation.qlldefining comparison, equality, and relational operations withgetGreaterOperand/getLesserOperand. - Updated
rust.qllimport and refactoredUncontrolledAllocationSizeExtensions.qllto use the new API. - Extended
test.rsandOperations.qlto include new tags (ComparisonOperation,EqualityOperation, specific operation classes, andGreater/Lesser).
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| rust/ql/lib/codeql/rust/elements/ComparisonOperation.qll | New file defining ComparisonOperation, its subtypes, and operand accessors. |
| rust/ql/lib/rust.qll | Imported ComparisonOperation into the Rust library root. |
| rust/ql/lib/codeql/rust/security/UncontrolledAllocationSizeExtensions.qll | Removed old operand helpers and switched to the new RelationalOperation API. |
| rust/ql/test/library-tests/operations/Operations.ql | Updated describe and test harness to recognize new operation tags. |
| rust/ql/test/library-tests/operations/test.rs | Added expected tags for comparison operations and Greater/Lesser values. |
| /** | ||
| * The equal comparison operation, `==`. | ||
| */ | ||
| final class EqualOperation extends EqualityOperationImpl, BinaryExpr { |
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.
BinaryExpr can be removed
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.
I think I prefer EqualityOperation.
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.
EqualityOperation is "an operation to do with equality", i.e. == or !=.
EqualOperation is specifically ==.
We can't use "EqualityOperation" to describe both.
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.
Removed redundant BinaryExpr.
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.
OK. Then I think I slightly prefer Equal_s_Operation over EqualOperation, but I'll let you decide.
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.
Yeah, I agree that's a better name. Updated.
rust/ql/lib/codeql/rust/security/UncontrolledAllocationSizeExtensions.qll
Outdated
Show resolved
Hide resolved
|
Thanks for the review suggestions. I've implemented all but one (see above). |
Adds a
ComparisonOperationlibrary, filling out more of theOperationclass tree.Cleans up some out of place predicates (
getGreaterOperandandgetLesserOperand) inUncontrolledAllocationSizeExtensions.qll.