Skip to content

Conversation

@paldepind
Copy link
Contributor

@paldepind paldepind commented Oct 4, 2025

This PR changes how bounds are adjusted for integral types.

As an example, if we have a strict lower bound > 1 on an integer then the lower bound is 2. As of now, this is achieved by adding 1 to the lower bound. However, this adjustment is not always correct.

For this guard, where e is an integer:

(2 * e + 1) > 0

We deduce a lower bound of > -0.5 and after adding 1 we get 0.5. For such a fractional lower bound adding 1 doesn't really make sense. This PR tweaks this slightly, such that the lower bound ends up being 0. A similar adjustment is made for upper bounds.

I ran into this when debugging a performance issue where this showed up. This adjustment didn't fix the performance issue, but I think it's still worthwhile.

Per-commit review is suggested — I also snuck in a few orthogonal changes.

I think the DCA report seems uneventful.

@github-actions github-actions bot added the C++ label Oct 4, 2025
@paldepind paldepind force-pushed the cpp/range-analysis-fix branch from a4d2455 to ba29a0a Compare October 4, 2025 14:59
@paldepind paldepind force-pushed the cpp/range-analysis-fix branch 2 times, most recently from 297f62d to 61f30c9 Compare October 5, 2025 09:41
}

/** Provides predicates for debugging the simple range analysis library. */
private module Debug {

Check warning

Code scanning / CodeQL

Dead code Warning

This code is never used, and it's not publicly exported.
@paldepind paldepind force-pushed the cpp/range-analysis-fix branch from 61f30c9 to c4f9212 Compare October 5, 2025 09:58
@paldepind paldepind changed the title C++: Range analysis fix C++: Range analysis guard improvement Oct 6, 2025
@paldepind paldepind marked this pull request as ready for review October 6, 2025 07:52
@paldepind paldepind requested a review from a team as a code owner October 6, 2025 07:52
@paldepind paldepind requested review from MathiasVP and Copilot October 6, 2025 07:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves how bounds are adjusted for integral types in C++ range analysis. Instead of simply adding/subtracting 1 to adjust bounds from fractional values like -0.5 to 0.5, it now properly handles the conversion to integer bounds by using floor operations to compute more accurate integer bounds.

Key changes:

  • Introduces new helper functions adjustLowerBoundIntegral and adjustUpperBoundIntegral for proper bound adjustment
  • Replaces simple arithmetic adjustment with floor-based calculations that handle fractional bounds correctly

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
SimpleRangeAnalysis.qll Adds new adjustment functions and updates bound calculation logic for integral types
RangeAnalysisUtils.qll Adds toString method to RelationStrictness class and improves documentation
test.c Adds new test cases for inequality-derived integer bounds
upperBound.expected Updated test expectations reflecting improved bound calculations
ternaryUpper.expected Updated test expectations for ternary expressions
ternaryLower.expected Updated test expectations for ternary expressions
lowerBound.expected Updated test expectations reflecting improved bound calculations

@paldepind paldepind added the no-change-note-required This PR does not need a change note label Oct 8, 2025
@paldepind
Copy link
Contributor Author

I'm putting this PR back in draft. I have an internal PR that adds new ceilFloat and floorFloat member predicates to float. If these are added we could do a cleaner implementation here.

@paldepind paldepind marked this pull request as draft October 13, 2025 09:48
@paldepind paldepind force-pushed the cpp/range-analysis-fix branch from c4f9212 to 3a13588 Compare January 9, 2026 09:26
@paldepind paldepind marked this pull request as ready for review January 9, 2026 13:32
@paldepind
Copy link
Contributor Author

I've rebased the PR and updated it to use the new ceilFloat and floorFloat predicates.

@paldepind paldepind requested review from andersfugmann and removed request for MathiasVP January 9, 2026 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C++ no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant