Skip to content

Conversation

@MathiasVP
Copy link
Contributor

In C/C++ there are many implicit conversions which must be handled as part of range analysis. For example, in the following:

void test(short s) {
  if(s < 5) { ... }
}

there's a short-to-int conversion on s. Previously, we made the new range analysis library handle this by wrapping the definition of a C/C++ SemExpr inside an EquivalenceClass so that the expression (int)s and the expression s was identical.

However, @aschackmull made me aware that the range analysis library already has support for "this expression reads this variable" to handle stuff like +n < 5 in Java. So we can get equivalent functionality by simply making these "safe conversions" copy-value expressions, and the range analysis library will ensure that a bound on (int)s in the example above is correctly identified as a bound on s.

cc @aschackmull what do you think of this? I know there are other changes you'd like to see to make your library sharing easier, but I think this is a good first step 😅.

@MathiasVP MathiasVP requested a review from a team as a code owner November 6, 2023 16:07
@github-actions github-actions bot added the C++ label Nov 6, 2023
@MathiasVP MathiasVP added the no-change-note-required This PR does not need a change note label Nov 6, 2023
@MathiasVP
Copy link
Contributor Author

DCA was uneventful 🎉

Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

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

If you are happy with the test changes, then I'd say the code changes LGTM!

@MathiasVP MathiasVP merged commit a04830b into github:main Nov 7, 2023
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.

2 participants