C++: A few small refactors to the simple range analysis library #20691
Conversation
This helps for debugging.
The proposition in the true branch implied the condition, so `or` is more appropriate. Also eliminated an existentially quantified variable.
The last disjunct in `boundFromGuard` is moved into `linearBoundFromGuard`. This avoids repeating the calculation for `boundValue`. `getBounds` and `getExprTypeBounds` are turned into predicates with result. Their middle argument was the "output" which was confusing.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the simple range analysis library in C++ with improvements to code clarity, consistency, and maintainability. The changes include correcting typos in comments, simplifying conditional logic, and restructuring helper predicates.
Key Changes:
- Simplified conditional expressions by converting if-then-else to disjunctions and using ternary operators where appropriate
- Refactored helper predicates to use result-returning functions instead of output parameters
- Added proper class wrapper for
RelationStrictnesstype with string representation
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test.c | Fixed spelling errors in comments ("guardPhy" → "guardPhi", "basic" → "basic block") |
| SimpleRangeAnalysis.qll | Refactored conditional logic, simplified predicates, and restructured bound computation helpers to improve code clarity |
| RangeAnalysisUtils.qll | Added RelationStrictness class wrapper with toString() method and improved documentation formatting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| upperBoundFromGuard(guard, access, guardUB, branch) and | ||
| defUB = getFullyConvertedUpperBounds(access) | ||
| | | ||
| defUB = getFullyConvertedUpperBounds(access) and |
There was a problem hiding this comment.
For what reason do you prefer and over | here?
There was a problem hiding this comment.
In my experience | is usually used for some reason. Either because there is an or afterwards or because the thing before and after are somehow different. Here there doesn't seem to be any reason, so going with the default and seems clearer :)
2 participants
A few inconsequential refactors that I did while working on #20645.
Going through each commit is probably easiest, as the commit descriptions have some details for most commits.