Skip to content

Rangeanalysis: Simplify Guards integration. #19571

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

Merged
merged 2 commits into from
May 26, 2025

Conversation

aschackmull
Copy link
Contributor

The Range Analysis library uses the second (_v2) Java Guards implication logic layer. This implementation detail is leaking all over, so this PR cleans that up by changing the interface to use the controls predicates that are closed under any suitable implication logic, and Java can then simply instantiate it with the proper guards layer without requiring the range analysis to know about implies_v2.
A small semantic effect is that "bound reasons" are now reported as the original comparison rather than the implied guard:

b = x < bound; // R1
...
if (b) { // R2
  use(x); // A
}

The reason for the bound on x at A is now reported as R1 instead of R2. I don't expect this to make much difference in practice.

@Copilot Copilot AI review requested due to automatic review settings May 23, 2025 11:48
@aschackmull aschackmull requested review from a team as code owners May 23, 2025 11:48
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label May 23, 2025
Copilot

This comment was marked as off-topic.

@aschackmull aschackmull requested a review from a team as a code owner May 23, 2025 12:17
@github-actions github-actions bot added the C# label May 23, 2025
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Looks plausible to me.

@aschackmull aschackmull merged commit a519eab into github:main May 26, 2025
46 checks passed
@aschackmull aschackmull deleted the rangeanalysis/guards branch May 26, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ Java 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.

2 participants