Rust: Add models for core::cmp::Ord::{min,max,clamp}#21035
Merged
hvitved merged 1 commit intogithub:mainfrom Dec 15, 2025
Merged
Rust: Add models for core::cmp::Ord::{min,max,clamp}#21035hvitved merged 1 commit intogithub:mainfrom
core::cmp::Ord::{min,max,clamp}#21035hvitved merged 1 commit intogithub:mainfrom
Conversation
351c295 to
fc49360
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds data flow models for Rust's core::cmp::Ord trait methods (min, max, and clamp), enabling CodeQL to track data flow through these comparison operations. The PR also includes comprehensive test cases to verify the models work correctly, particularly with custom dereferencing scenarios.
Key Changes:
- Added flow models for
Ord::{min,max,clamp}to track that values flow from both/all arguments to the return value - Added new test case
test_ord()demonstrating flow through these methods - Expanded some if-else blocks in existing code for better readability (formatting change only)
- Updated test expectations to reflect the new models
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
rust/ql/lib/codeql/rust/frameworks/stdlib/core.model.yml |
Adds three new flow models for min, max, and clamp methods |
rust/ql/test/library-tests/dataflow/models/models.ext.yml |
Removes test model for max (now covered by main models) |
rust/ql/test/library-tests/dataflow/models/models.expected |
Updates expected test output to show Argument[self,0] instead of Argument[self] |
rust/ql/test/library-tests/dataflow/modeled/main.rs |
Adds test_ord() function with test cases for all three methods |
rust/ql/test/library-tests/dataflow/modeled/inline-flow.expected |
Updates expected test results with new flow paths |
rust/ql/test/library-tests/dataflow/global/main.rs |
Expands if-else formatting and adds a.min(1042) test case |
rust/ql/test/library-tests/dataflow/global/inline-flow.expected |
Updates expected results reflecting line number changes |
rust/ql/test/library-tests/dataflow/global/viableCallable.expected |
Updates expected results reflecting line number changes |
rust/ql/test/library-tests/dataflow/global/CONSISTENCY/PathResolutionConsistency.expected |
Updates line numbers for consistency checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
geoffw0
approved these changes
Dec 15, 2025
| # Ord | ||
| - ["<_ as core::cmp::Ord>::min", "Argument[self,0]", "ReturnValue", "value", "manual"] | ||
| - ["<_ as core::cmp::Ord>::max", "Argument[self,0]", "ReturnValue", "value", "manual"] | ||
| - ["<_ as core::cmp::Ord>::clamp", "Argument[self,0,1]", "ReturnValue", "value", "manual"] |
Contributor
There was a problem hiding this comment.
Oh its nice to see Rust has a clamp method (and we're modelling it).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As part of #20987, I wanted to write a data flow test using implicit custom dereferencing, and found out that we are lacking flow models for
Ordmethods. This PR adds those models, as well as the test case.