Revert "C++: Range analysis measure bounds" #20774
Closed
Conversation
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
Otherwise the test output changes when unrelated models are added.
Moves the existing points-to predicates to the newly added class `ControlFlowNodeWithPointsTo` which resides in the `LegacyPointsTo` module. (Existing code that uses these predicates should import this module, and references to `ControlFlowNode` should be changed to `ControlFlowNodeWithPointsTo`.) Also updates all existing points-to based code to do just this.
This had only two uses in our libraries, so I simply inlined the predicate body in both places.
I wasn't entirely sure if this should be classified as `deprecated` or `breaking`, but seeing as these changes technically _could_ break existing queries (requiring a small rewrite), I opted for the latter.
Co-authored-by: hvitved <3667920+hvitved@users.noreply.github.com>
Co-authored-by: hvitved <3667920+hvitved@users.noreply.github.com>
Co-authored-by: Tom Hvitved <hvitved@github.com>
Co-authored-by: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com>
Rust: Handle variables introduced in if-let guards
CODEOWNERS: Add code-scanning-language-coverage team to all extractors
Add 'code-quality-extended' to query packs list
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request removes the bounds estimation optimization from the simple range analysis library, which was designed to prevent combinatorial explosion by widening when too many bounds were predicted.
- Removed the
BoundsEstimatemodule that estimated the number of bounds - Simplified widening to only apply to recursive expressions, not estimated-large expressions
- Removed pathological test cases that tested the bounds estimation feature
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/lib/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll | Removed BoundsEstimate module, widenLowerBound/widenUpperBound helper functions, and debug module; simplified widening conditions to only check isRecursiveBinary/isRecursiveDef |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/test.c | Removed repeated_if_statements, conditional_nested_guards, and many_conditional_assignments test functions that tested bounds estimation |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/nrOfBounds.ql | Deleted test query for estimating number of bounds |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/upperBound.expected | Updated expected results after removing test cases (auto-generated) |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/lowerBound.expected | Updated expected results after removing test cases (auto-generated) |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/ternaryUpper.expected | Updated expected results after removing test cases (auto-generated) |
| cpp/ql/test/library-tests/rangeanalysis/SimpleRangeAnalysis/ternaryLower.expected | Updated expected results after removing test cases (auto-generated) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
This needs to target the release branch, not |
Contributor
Author
|
Wrong revert as it targets the wrong branch. Will close and reopen it correctly |
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.
Reverts #20645
Reverting as this causes stable timeout regressions in projects with different sizes.